Conversation
| def _handle_backfill_alerts(self): | ||
| # Get backfilled alerts | ||
| bugzilla = BugzillaHelper() | ||
| backfilled_records = BackfillRecord.objects.select_related( |
There was a problem hiding this comment.
I think we can extract this as a method of BackfillRecord.
| return | ||
|
|
||
| @staticmethod | ||
| def recompute_backfill_alert(record: BackfillRecord) -> Push: |
There was a problem hiding this comment.
The type hint for the output isn't correct.
| else: | ||
| logger.info(f"Change not found here {prev} vs. {cur}") | ||
|
|
||
| if not first_changed: |
There was a problem hiding this comment.
I'm not sure what to say about this. So far we've only marked invalid alerts manually.
Trying to automate this can be risky & end up hiding real regressions.
There was a problem hiding this comment.
Agree. I wouldn't go that far.
| # TODO: get job_type from record when soft launch lands ---> job_type = record.job_type | ||
| job_type = get_job_type(record) | ||
| from_time, to_time = self._get_push_timestamp_range(record.get_context()) | ||
| from_time, to_time = Helper.get_push_timestamp_range(record.get_context()) |
There was a problem hiding this comment.
There's no need to refactor OutcomeChecker, as we already covered these exact methods in PR 7070.
|
|
||
| # Check if this push has an alert summary already. If it does, reassign | ||
| # the alerts to it, delete this summary. | ||
| existing_summary = PerformanceAlertSummary.objects.filter(push=cur_push) |
There was a problem hiding this comment.
PerformanceAlertSummary.objects.filter(push=cur_push).first() gets the first one or None if there is no result after filtering
|
The idea of this bot is awesome, but is pretty tricky and complex to implement. I am saying this from the experience of developing the automated bug filling from the alerts view. |
|
is there planned work on this PR? I am trying to close out old PRs or ensure they have a valid project and will be worked on in the near future. |
|
@jmaher you can close it. We can find it in the closed PRs if we need it again. |
5 participants
@gmierz I created this PR to easily compare the changes and leave some notes