ref(ACI): Update evaluate_value return #92439
Conversation
github-actions
bot
added
the
Scope: Backend
src/sentry/workflow_engine/handlers/condition/anomaly_detection_handler.py
Outdated
Show resolved
Hide resolved
src/sentry/workflow_engine/handlers/condition/anomaly_detection_handler.py
Outdated
Show resolved
Hide resolved
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 #92439 +/- ##
==========================================
- Coverage 87.89% 87.89% -0.01%
==========================================
Files 10250 10250
Lines 587913 587931 +18
Branches 22828 22828
==========================================
+ Hits 516774 516784 +10
- Misses 70693 70701 +8
Partials 446 446 |
There was a problem hiding this comment.
i don't think we should be introducing any anomaly detection code with the change to evaluate_value -- instead we can make a mock data condition that meets the criteria we want and returns an overriding result. this will allow us to change the result and create different assertions and not have to introduce the AD code.
| DataConditionHandler, | ||
| DetectorPriorityLevel, | ||
| WorkflowEventData, | ||
| ) | ||
|
|
||
|
|
||
| @condition_handler_registry.register(Condition.ANOMALY_DETECTION) | ||
| class AnomalyDetectionHandler(DataConditionHandler[WorkflowEventData]): |
There was a problem hiding this comment.
🤔 - side note -- i didn't realize we have a seer module already - thoughts on this condition living there? that way it can be along-side the AnomalyDetection* definitions as well.
There was a problem hiding this comment.
Yeah I can move the file in the other PR 👍
| type=Condition.ANOMALY_DETECTION, | ||
| comparison={ | ||
| "sensitivity": AnomalyDetectionSensitivity.MEDIUM, | ||
| "seasonality": AnomalyDetectionSeasonality.AUTO, | ||
| "threshold_type": AnomalyDetectionThresholdType.ABOVE_AND_BELOW, |
There was a problem hiding this comment.
it'd be nice to separate this info out so we can show the simplest working verisons of this condition, then in tests for the AnomalyDetectionConditionHandler we can evaluate this case. that will also help reduce the number of dependencies in the test and increase the speed of each test evaluation 🎉
src/sentry/workflow_engine/handlers/condition/anomaly_detection_handler.py
Outdated
Show resolved
Hide resolved
ceorourke
force-pushed
the
ceorourke/update-evaluation-result-return
branch
from
db4d611 to
0b5b9fb
Compare
ceorourke
force-pushed
the
ceorourke/update-evaluation-result-return
branch
from
86e8e40 to
5a47707
Compare
ceorourke
force-pushed
the
ceorourke/update-evaluation-result-return
branch
from
1df4ea9 to
3d2e062
Compare
An offshoot of #88647 to update `evaluate_value` to handle when the handler returns a result that isn't a boolean as we will for anomaly detection.
An offshoot of #88647 to update
evaluate_valueto handle when the handler returns a result that isn't a boolean as we will for anomaly detection.