-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
fix(aci): Use filter_recently_fired_workflow_actions exclusively #93382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yay
There's an argument that we should've tracked the differences in outcome and validated that they are what we intend, but given the lack of validated traffic on the old implementation, I'm not sure that's worth waiting for vs just trying to make sure this is correct. |
Actually, I should probably remove the various lingering uses of |
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 #93382 +/- ##
==========================================
+ Coverage 83.38% 88.07% +4.68%
==========================================
Files 10322 10311 -11
Lines 595614 594638 -976
Branches 23137 22985 -152
==========================================
+ Hits 496637 523707 +27070
+ Misses 98484 70499 -27985
+ Partials 493 432 -61 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love this simplification! 🎉
src/sentry/incidents/endpoints/serializers/workflow_engine_detector.py
Outdated
Show resolved
Hide resolved
@@ -7,6 +7,7 @@ | |||
@region_silo_model | |||
class ActionGroupStatus(DefaultFieldsModel): | |||
""" | |||
DEPRECATED: Use WorkflowActionGroupStatus instead. This will be removed soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! 🥇
) Use WorkflowActionGroupStatus for our recency filtering rather than ActionGroupStatus. While we're at it, remove any remaining active uses of ActionGroupStatus. --------- Co-authored-by: Josh Callender <[email protected]> Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
) Use WorkflowActionGroupStatus for our recency filtering rather than ActionGroupStatus. While we're at it, remove any remaining active uses of ActionGroupStatus. --------- Co-authored-by: Josh Callender <[email protected]> Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Use WorkflowActionGroupStatus for our recency filtering rather than ActionGroupStatus.
While we're at it, remove any remaining active uses of ActionGroupStatus.