bundle analysis: update trend measurement compute #632
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #632 +/- ##
=======================================
Coverage 91.52% 91.53%
=======================================
Files 621 621
Lines 16496 16515 +19
=======================================
+ Hits 15098 15117 +19
Misses 1398 1398
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #632 +/- ##
=======================================
Coverage 91.52% 91.53%
=======================================
Files 621 621
Lines 16496 16515 +19
=======================================
+ Hits 15098 15117 +19
Misses 1398 1398
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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 #632 +/- ##
===========================================
Coverage 95.94000 95.94000
===========================================
Files 799 799
Lines 17813 17832 +19
===========================================
+ Hits 17090 17109 +19
Misses 723 723
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
graphql_api/actions/measurements.py
Outdated
| @@ -44,6 +44,28 @@ def measurements_by_ids( | |||
| return measurements | |||
|
|
|||
|
|
|||
| def measurements_last_uploaded_by_before_date( | |||
There was a problem hiding this comment.
a bit confused w/ the naming here since we're using start_date in the timestamp, wouldn't these be measurements_before_start_date?
There was a problem hiding this comment.
Makes sense, renamed.
services/bundle_analysis.py
Outdated
| branch=self.branch, | ||
| ) | ||
|
|
||
| print("what on earth", all_measurements) |
services/bundle_analysis.py
Outdated
|
|
||
| # There isn't any measurements before the start date range, it will just be null | ||
| if not carryover_measurement: | ||
| continue |
There was a problem hiding this comment.
Nit: change this to "if measurements: ..." and get rid of the continue/1 line of code
Edit: oh wait you want to keep going if there isn't a measurement, ignore the above then. I gues the question is, you still want to save a carryover with null values?
There was a problem hiding this comment.
No need to save a carryover with null values because there's a helper function that populations non-existent measurements with null values within the query date range.
services/bundle_analysis.py
Outdated
| # Create a new datapoint in the measurements and prepend it to the existing list | ||
| # If there isn't any measurements before the start date range, measurements will be untouched | ||
| if carryover_measurement: | ||
| value = ( |
There was a problem hiding this comment.
This could be shortened too since the conditional is met above 😬
closes: codecov/engineering-team#1976
Make it so that when fetching measurements for trends, if the start date range doesn't have any value, then we look back further until we find the first instance of a measurement point. Then we use that point's value as the value of the start date range. Otherwise it becomes ambiguous if the measurement ID has no measurement at all in history or that the start date has no measurement.
With this if the start date range is still null it means there was never any measurements before the start date. If the start date range is not null there were measurements carried over from the further past.
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.