feat(ACI): Enable/disable detectors #93782
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #93782 +/- ##
==========================================
+ Coverage 81.66% 88.04% +6.38%
==========================================
Files 10330 10339 +9
Lines 596288 596901 +613
Branches 23163 23163
==========================================
+ Hits 486957 525563 +38606
+ Misses 108875 70882 -37993
Partials 456 456 |
| @@ -1003,7 +1003,7 @@ def update_alert_rule( | |||
| return alert_rule | |||
|
|
|||
|
|
|||
| def update_detector(alert_rule: AlertRule, enabled: bool) -> None: | |||
| def update_dual_written_detector(alert_rule: AlertRule, enabled: bool) -> None: | |||
There was a problem hiding this comment.
should this method also be setting the enabled property on the detector since that's the field currently being used to filter detectors? (at least until we figure out all the status stuff)
I wasn't sure if this was being invoked after that was set or what not.
There was a problem hiding this comment.
So https://github.com/getsentry/sentry/pull/93673/files was done incorrectly? Should I just change this all to enabled and not touch status?
There was a problem hiding this comment.
the field the platform uses is enabled -- from office hours yesterday it sounds like that might change (thread discussing it as well).
I think for now, it's safest to set both -- until we can sort it all out.
| ) | ||
|
|
||
| detector.refresh_from_db() | ||
| assert detector.status == detector_status |
There was a problem hiding this comment.
should this also assert detector.enabled is True?
Follow up to #93673 and precursor to https://github.com/getsentry/getsentry/pull/17739 to enable and disable detectors and the related query subscriptions.