ref(escalating-issues): Update cron task to use nodestore #47496
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #47496 +/- ##
===========================================
+ Coverage 63.78% 80.59% +16.80%
===========================================
Files 4729 4762 +33
Lines 200896 201431 +535
Branches 11618 11626 +8
===========================================
+ Hits 128144 162341 +34197
+ Misses 72496 38832 -33664
- Partials 256 258 +2
|
| escalating_group_forecast.save() | ||
|
|
||
|
|
||
| def get_forecasts(groups: List[Group]) -> None: |
There was a problem hiding this comment.
This function will be called by Snigdha's PR, thus, I would like to decouple it from the weekly task module.
Could you please open a new PR moving this function and save_forecast_per_group to src/sentry/issues/escalating.py? (I believe that's a good centralized place) or even src/sentry/issues/forecasts.py if it makes more sense. Feel free to propose a better location.
There was a problem hiding this comment.
Thank you for the suggestion, here's the PR
| fetched_forecast = EscalatingGroupForecast.fetch(group_list[0].project.id, group_list[0].id) | ||
| assert fetched_forecast.project_id == group_list[0].project.id | ||
| assert fetched_forecast.group_id == group_list[0].id | ||
| assert fetched_forecast.forecast == FORECAST_LIST_MOCK |
There was a problem hiding this comment.
I'm all confused. Why is the forecast 14 times the number 200?
There was a problem hiding this comment.
The forecast is [200] * 14 because I didn't add a lot of data from the past week in the tests, and it defaults to 200 when this happens (see here). I figured the tests for the algorithm itself should suffice to actually test the forecast values
There was a problem hiding this comment.
Okay. This is low end forecast. Thank you for clarifying.
|
|
||
| # Assert no duplicates when this is run twice | ||
| # Assert update when this is run twice |
There was a problem hiding this comment.
This is probably fine, however, we should have it in mind if re-runs of the tasks happened often and we wanted the re-runs to be faster.
Co-authored-by: Armen Zambrano G. <armenzg@sentry.io>
Update escalating issues forecast cron task to use nodestore instead of GroupForecast
WOR-2968