-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ci_service field to upload endpoint #436
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 #436 +/- ##
=======================================
Coverage 96.04% 96.04%
=======================================
Files 643 643
Lines 17055 17057 +2
=======================================
+ Hits 16380 16382 +2
Misses 675 675
Flags with carried forward coverage won't be shown. Click here to find out more.
|
upload/serializers.py
Outdated
if "version" in validated_data.keys(): | ||
validated_data.pop("version") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: there is a default
argument on dict.pop()
, you can do this in one step https://python-reference.readthedocs.io/en/latest/docs/dict/pop.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose I could do something like
validated_data.pop("version", default=None)
but I don't think it's clear that default
is necessary or else this will throw an error if
version is not found
would it be cleaner to do what's below or to do what's above with a comment explaining that default is necessary?
try:
validated_data.pop("version", default=None)
except KeyError:
pass
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personally i think validated_data.pop("version", default=None)
is clear enough. you don't assign the return value to anything so it's just getting rid of the value if it's in there
there's room to disagree there, though. if you want to be more explicit, i think tweak the current approach:
if "version" in validated_data:
validated_data.pop("version")
i think the above is more "pythonic" + probably slightly more efficient (not that it matters here). checking for it in .keys()
is probably O(n), but key in dict
is probably O(1) haha
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #436 +/- ##
=======================================
Coverage 96.04% 96.04%
=======================================
Files 643 643
Lines 17055 17057 +2
=======================================
+ Hits 16380 16382 +2
Misses 675 675
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #436 +/- ##
=====================================
Coverage 95.75 95.75
=====================================
Files 765 765
Lines 17641 17643 +2
=====================================
+ Hits 16892 16894 +2
Misses 749 749
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io>
1040668
to
8ed9ae3
Compare
Links to relevant tickets
Fixes: codecov/engineering-team#1297
What does this PR do?
UploadSerializer
that iswrite_only