chore(aci): handle workflows or rules in integrations sans feature flag #106048
Conversation
github-actions
bot
added
the
Scope: Backend
cathteng
changed the title
| ) | ||
| ) | ||
| key, value = get_rule_or_workflow_id(rules[0]) | ||
| if key == "workflow_id": |
There was a problem hiding this comment.
This is where variant types would be nice..
it could be like
match get_rule_or_workflow_id(rules[0]):
case WorkflowId(value):
rule_url = ..
case LegacyRuleId(_):
rule_url = build_rule_url(...)
| self.issue_details, | ||
| self.notification, | ||
| ExternalProviders.SLACK, | ||
| rule_id, |
There was a problem hiding this comment.
this is supposed to be the workflow_id?
There was a problem hiding this comment.
the rule_id is set to whatever value we end up picking in the logic above (it used to work like that and just keeping it for now)
|
|
||
| rule_id = None | ||
| rule_environment_id = None | ||
| key = "legacy_rule_id" |
There was a problem hiding this comment.
maybe 'rule_id_type'? "key" is pretty vague.
|
|
||
|
|
||
| def get_rule_or_workflow_id(rule: Rule) -> tuple[str, str]: | ||
| def get_rule_or_workflow_id(rule: Rule) -> tuple[RuleIdType, str]: |
There was a problem hiding this comment.
Wouldn't tuple[RuleIdType, int] be more accurate and cleaner here?
| obj: Group | GroupEvent = self.event if self.event is not None else self.group | ||
| rule_id = None | ||
| rule_environment_id = None | ||
| key: RuleIdType = "legacy_rule_id" |
There was a problem hiding this comment.
thought: should we have a DEFAULT_RULE_ID_TYPE const to avoid needing to annotate and so this choice is uniform, obvious, and easy to change?
There was a problem hiding this comment.
Agree, we should have an easy way to flip the default after GA to always read the workflow id
cathteng
merged commit c91f208
into
cathy/aci/workflow-rule-in-digests
Update integrations code to handle using legacy_rule_id or workflow_id depending on if it exists. We prefer using legacy_rule_id if available, otherwise try workflow_id, then finally use rule.id
Builds on #105999