feat(workflow_engine): Add in hook for producing occurrences from the stateful detector #7
Conversation
… stateful detector (#80168) This adds a hook that can be implemented to produce an occurrence specific to the detector that is subclassing the StatefulDetector. Also change the signature of evaluate to return a dict keyed by groupkey instead of a list. This helps avoid the chance of duplicate results for the same group key. <!-- Describe your PR here. -->
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) -> list[DetectorEvaluationResult]: | ||
| # TODO: Implement | ||
| return [] | ||
| class MetricAlertDetectorHandler(StatefulDetectorHandler[QuerySubscriptionUpdate]): |
There was a problem hiding this comment.
The MetricAlertDetectorHandler class now inherits from StatefulDetectorHandler but provides no implementation for the required abstract methods (get_dedupe_value, get_group_key_values, and build_occurrence_and_event_data). This will cause runtime errors when instantiated. Either provide concrete implementations or add a comment explaining why this is intentionally incomplete.
| class MetricAlertDetectorHandler(StatefulDetectorHandler[QuerySubscriptionUpdate]): | |
| class MetricAlertDetectorHandler(StatefulDetectorHandler[QuerySubscriptionUpdate]): | |
| """ | |
| Placeholder for a future stateful detector handler for metric alerts. | |
| This class intentionally does not implement the required abstract methods | |
| (get_dedupe_value, get_group_key_values, build_occurrence_and_event_data) | |
| and should not be instantiated until the abstraction is complete. | |
| """ |
| event_data, | ||
| ) | ||
| occurrence_2, event_data_2 = build_mock_occurrence_and_event( | ||
| detector.detector_handler, "group_2", 6, PriorityLevel.HIGH |
There was a problem hiding this comment.
The value parameter passed to build_mock_occurrence_and_event should be 10, not 6, to match the group_vals data packet on line 177 where 'group_2': 10. This inconsistency could mask bugs in the occurrence generation logic.
| detector.detector_handler, "group_2", 6, PriorityLevel.HIGH | |
| detector.detector_handler, "group_2", 10, PriorityLevel.HIGH |
| self.enqueue_state_update(group_key, is_active, new_status) | ||
| event_data = None | ||
| result = None | ||
| result: StatusChangeMessage | IssueOccurrence |
There was a problem hiding this comment.
[nitpick] The type annotation for 'result' is redundant since it's immediately assigned in both branches of the if-else statement. Consider removing this line or initializing result with a default value.
| result: StatusChangeMessage | IssueOccurrence | |
| result |
Test 10
Replicated from ai-code-review-evaluation/sentry-copilot#9