REST API: add flag coverage to flag list endpoint #765
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #765 +/- ##
================================================
+ Coverage 96.14000 96.15000 +0.01000
================================================
Files 812 812
Lines 18400 18416 +16
================================================
+ Hits 17691 17707 +16
Misses 709 709
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 ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
| ] | ||
| try: | ||
| report = self.get_commit().full_report | ||
| if not report: |
There was a problem hiding this comment.
just curious, when would we throw a NotFound vs. return None?
There was a problem hiding this comment.
this line would return the list of flags without the coverage information for each flag if the coverage report of the head commit is not yet processed.
in general imo we should throw 404 if the requested data is not found (eg in the param you have a repo/owner that doesn't exist) so there's nothing to return. And return null, I don't think we would return that as a response to the entire request, rather the response object has some partial data is not available.
There was a problem hiding this comment.
Gotcha! Makes sense to me, thanks for the context
|
|
||
| for i, val in enumerate(results): | ||
| flag_report = report.filter(flags=[val["flag_name"]]) | ||
| results[i]["coverage"] = flag_report.totals.coverage or 0 |
There was a problem hiding this comment.
totals will always exist?
There was a problem hiding this comment.
from what I can tell yes
| "next": None, | ||
| "previous": None, | ||
| "results": [ | ||
| {"flag_name": "foo", "coverage": 0}, |
There was a problem hiding this comment.
Jw on returning 0 for coverage here vs. None 🤔
There was a problem hiding this comment.
yeah I feel none is better
closes codecov/feedback#362
Updates
/api/v2/{service}/{owner_username}/repos/{repo_name}/flags/endpoint to include the coverage value for each flag in the response object.Old:
"results": [{"flag_name": string}]New:
"results": [{"flag_name": string, "coverage": float}]This coverage value comes from the computed report of the head commit of the repo's default branch. If failure to retrieve such value a default value of 0 will be returned.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.