ref(workflow_engine): Refactor StatefulDetectorHandler + StatefulGroupingDetectorHandler #92457
Conversation
github-actions
bot
added
the
Scope: Backend
| @@ -10,13 +10,6 @@ class QuerySubscriptionUpdate(TypedDict): | |||
| timestamp: datetime | |||
|
|
|||
|
|
|||
| class MetricDetectorUpdate(TypedDict): | |||
There was a problem hiding this comment.
sorry i snuck this into the pr, i noticed it's redundant with QuerySubscriptionUpdate and updated the use (it just ended up being a few lines)
| if self._is_detector_group_value(data_values): | ||
| group_data_values = cast(dict[DetectorGroupKey, DataPacketEvaluationType], data_values) | ||
| else: | ||
| group_data_values = {None: cast(DataPacketEvaluationType, data_values)} |
There was a problem hiding this comment.
is there a better way to handle this w/o using cast? i couldn't figure out anything...
There was a problem hiding this comment.
Yes I think you can inform the type system that your _is_detector_group_value function is doing discrimination using a TypeGuard
saponifi3d
force-pushed
the
jcallender/aci/stateful-cleanup-2
branch
from
129cb23 to
6098396
Compare
saponifi3d
changed the title
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #92457 +/- ##
===========================================
+ Coverage 45.71% 87.89% +42.17%
===========================================
Files 10216 10232 +16
Lines 585548 586279 +731
Branches 22785 22785
===========================================
+ Hits 267687 515299 +247612
+ Misses 317431 70550 -246881
Partials 430 430 |
| event_data=event_data, | ||
| ) | ||
|
|
||
| def _is_detector_group_value(self, value) -> bool: |
There was a problem hiding this comment.
is there another way i can tell if this is a DataPacketEvaluationType or dict[DetectorGroupKey, DataPacketEvaluationType]? (i can't use isinstance on DataPacketEvaluationType because it's generic 😭)
saponifi3d
force-pushed
the
jcallender/aci/stateful-cleanup-2
branch
from
c5833ea to
2a2614c
Compare
saponifi3d
force-pushed
the
jcallender/aci/stateful-cleanup-2
branch
from
ccbca69 to
798fc67
Compare
Description
nonea lot for the shared helpers. So refactored this to merge them a bit more together, this will take away the difficulty for implementing a stateful detector handler, but not reduce all the complexity in the class (seems like a reasonable tradeoff for now)