Refactor gaps and empty shift checks #3785
Conversation
Ferril
added
the
pr:no public docs
| @@ -84,6 +88,8 @@ def refresh_ical_file(schedule_pk): | |||
| # update cached schedule on-call users | |||
| update_cached_oncall_users_for_schedule(schedule) | |||
|
|
|||
| check_gaps_and_empty_shifts_in_schedule.apply_async((schedule_pk,)) | |||
|
|
|||
| if run_task: | |||
| notify_about_empty_shifts_in_schedule_task.apply_async((schedule_pk,)) | |||
There was a problem hiding this comment.
Wondering if we should queue these 2 tasks after we are sure the gaps/empty shifts were updated?
There was a problem hiding this comment.
These tasks are not about updating these fields, they notify users in Slack about gaps and empty shifts if ical was changed
There was a problem hiding this comment.
Hm.. but these tasks, when run, will re-check for gaps and empty shifts (and update the fields too, it seems), so in some cases we will be doing the work twice?
There was a problem hiding this comment.
yeah... 🤔 but I don't see how to avoid this without more detailed refactoring, since these notfify tasks use different types of events with different fields: ScheduleEvent and EmptyShifts. For regular update I use ScheduleEvent for both checks and parse calendars only once because we don't need an additional information about empty events there.
There was a problem hiding this comment.
Ok, makes sense. I would at least leave some comment/TODO/issue that we would like to refactor this at some point.
Refactor gaps and empty shift checks: - Increase checking gaps and empty shifts frequency - Unify gaps and empty shift checks
What this PR does
Which issue(s) this PR fixes
Related to https://github.com/grafana/support-escalations/issues/9116
Checklist
pr:no public docsPR label added if not required)CHANGELOG.mdupdated (orpr:no changelogPR label added if not required)