fix: use get_or_create in CommitSerializer.create #437
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
@@ Coverage Diff @@
## main #437 +/- ##
=======================================
Coverage ? 96.05%
=======================================
Files ? 643
Lines ? 17060
Branches ? 0
=======================================
Hits ? 16387
Misses ? 673
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
I was looking at mentions of CommitSerializer in the unit tests, but could only find tests in which the create method are not called.
In tests/views/test_commits.py::test_create_commit_already_exists I don't see an assert to make sure that a duplicated Commit was not created.
Can you add a test that calls the CommitSerializer.create method twice and/or asserts that no duplicates happen?
| if commit: | ||
| return commit | ||
| return super().create(validated_data) | ||
| repo = validated_data.pop("repository", None) |
There was a problem hiding this comment.
Are you sure this is a Repository and not RepositorySerializer?
There was a problem hiding this comment.
perfection, thank you for checking that
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #437 +/- ##
=======================================
Coverage ? 96.05%
=======================================
Files ? 643
Lines ? 17060
Branches ? 0
=======================================
Hits ? 16387
Misses ? 673
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| if commit: | ||
| return commit | ||
| return super().create(validated_data) | ||
| repo = validated_data.pop("repository", None) |
There was a problem hiding this comment.
perfection, thank you for checking that
there is a possibility of creating commits concurrently if that happens there is a possibility of getting an IntegrityError when trying to create the Commit object this commit solves this problem by using get_or_create to create the Commit object in the serializer Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io>
joseph-sentry
force-pushed
the
joseph/get-or-create-commit
branch
from
4204a58
to
b5e9f0e
Compare
* fix: use get_or_create in CommitSerializer.create there is a possibility of creating commits concurrently if that happens there is a possibility of getting an IntegrityError when trying to create the Commit object this commit solves this problem by using get_or_create to create the Commit object in the serializer * test: add test to make sure CommitSerializer create only creates one object in the db --------- Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io>
* Test with states * second round * Okta stuff * quick fix for test * fixing tests * more tests * test: fix query measurements after 30 days test (#442) * test: fix query measurements after 30 days test This test started failing ater 2024-03-10 because the time at the query was not frozen to a specific date so it was using the system's actual time and the measurements were not a part of the last 30 days. * fix: add comments to clarify test fix Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io> * [admin] allow trial extension for orgs currently on trial (#438) Currently the admin app can extend a org trial, but only after they have expired and turned back to basic plan. This change allows the orgs' trials to be extended while they're still trialing. * Bundle Analysis: delete old code (#444) These fields are being deprecated by bundleData and bundleChange. The app is no longer calling these fields anymore, it is safe to remove now. * fix: use get_or_create in CommitSerializer.create (#437) * fix: use get_or_create in CommitSerializer.create there is a possibility of creating commits concurrently if that happens there is a possibility of getting an IntegrityError when trying to create the Commit object this commit solves this problem by using get_or_create to create the Commit object in the serializer * test: add test to make sure CommitSerializer create only creates one object in the db --------- Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io> * Revert "Bundle Analysis: delete old code (#444)" (#445) This reverts commit 7932e93. * Remove state after login --------- Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io> Co-authored-by: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Co-authored-by: JerrySentry <142266253+JerrySentry@users.noreply.github.com>
Purpose/Motivation
there is a possibility of creating commits concurrently if that happens there is a possibility of getting an IntegrityError when trying to create the Commit object
this commit solves this problem by using get_or_create to create the Commit object in the serializer
What does this PR do?
get_or_createinstead offilterthensuper().create