-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
ref(aci): pass WorkflowJob into process_workflows #82096
Conversation
80a3b78
to
94a8900
Compare
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #82096 +/- ##
==========================================
- Coverage 80.37% 80.36% -0.01%
==========================================
Files 7277 7277
Lines 321488 321487 -1
Branches 20966 20966
==========================================
- Hits 258391 258377 -14
- Misses 62689 62702 +13
Partials 408 408 |
f9840b8
to
0acfef2
Compare
@@ -14,11 +14,13 @@ | |||
|
|||
|
|||
# TODO - cache these by evt.group_id? :thinking: | |||
def get_detector_by_event(evt: GroupEvent) -> Detector: | |||
issue_occurrence = evt.occurrence | |||
def get_detector_by_event(job: PostProcessJob) -> Detector: |
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.
nit: get_detector_by_job
?
def get_detector_by_event(evt: GroupEvent) -> Detector: | ||
issue_occurrence = evt.occurrence | ||
def get_detector_by_event(job: PostProcessJob) -> Detector: | ||
issue_occurrence = job["event"].occurrence |
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.
could be nice to pull this off event = job["event"]
since you do that twice
too many merge conflicts, replaced with #82489 |
process_workflows
is currently called with aGroupEvent
for issue types like metric issues to fire downstream actions.To process issue alerts, we use other elements of
PostProcessJob
in post_process to evaluate conditions. As such, we should passPostProcessJob
intoprocess_workflows
. It contains theGroupEvent
.PostProcessJob
is aTypedDict
which also has the benefit of allowing us to modify the input without having to refactor stuff that's already in place.