Skip to content
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 #82489

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

cathteng
Copy link
Member

Original PR #82096

process_workflows is currently called with a GroupEvent 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 pass a version of PostProcessJob (here called WorkflowJob) into process_workflows. It contains a required GroupEvent and then makes everything else from PostProcessJob optional.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Dec 20, 2024
@cathteng cathteng requested a review from a team December 20, 2024 19:43
@cathteng cathteng marked this pull request as ready for review December 20, 2024 19:43
@cathteng cathteng requested a review from a team as a code owner December 20, 2024 19:43
Comment on lines +1006 to +1015
# PostProcessJob event is optional, WorkflowJob event is required
if "event" not in job:
logger.error("Missing event to create WorkflowJob", extra={"job": job})
return

try:
workflow_job = WorkflowJob({**job}) # type: ignore[typeddict-item]
except Exception:
logger.exception("Could not create WorkflowJob", extra={"job": job})
return
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't make mypy happy with this, it kept complaining the event didn't exist

Comment on lines +32 to +43
class EventJob(TypedDict):
event: GroupEvent


class WorkflowJob(EventJob, total=False):
group_state: GroupState
is_reprocessed: bool
has_reappeared: bool
has_alert: bool
has_escalated: bool


Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the main piece of this PR

if state is None:
return False

return state["is_regression"] == comparison
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is is_regression guaranteed to exist on state? if not I'd do a .get()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's guaranteed, the state is a typed dict too

class GroupState(TypedDict):
id: int
is_new: bool
is_regression: bool
is_new_group_environment: bool

@cathteng cathteng merged commit fe35f16 into master Dec 20, 2024
49 checks passed
@cathteng cathteng deleted the cathy/aci/workflow-job-dict branch December 20, 2024 20:00
@github-actions github-actions bot locked and limited conversation to collaborators Jan 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants