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

feat(workflow_engine): Add a DataConditionHandlerType to each DataConditionHandler #83276

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

saponifi3d
Copy link
Contributor

Description

As we were implementing DataConditionHandlers, I realized we didn't have a way to filter each handler in an API for the UI. This allow users to select the type of handler they want, and then configure it.

To address this, adding a static type to the condition handlers to match where in the product they'll be used;

  • detector triggers (used for setting thresholds and then results give detector states)
  • workflow triggers (used for workflow conditions, to evaluate if the issues are new / regressed, etc)
  • action filters (used to decide if we want to invoke an action in a workflow or not, if a tag is set or a priority needs to be met)

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 10, 2025
…he handlers in APIs, allowing us to generate dropdowns for each product area (detectors, workflows, actions)
@saponifi3d saponifi3d force-pushed the jcallender/aci/data-conditions-api-spike branch 2 times, most recently from c2d790a to f39a0b5 Compare January 10, 2025 22:35
…will allow us to have filtered dropdowns in the ui
@saponifi3d saponifi3d force-pushed the jcallender/aci/data-conditions-api-spike branch from f39a0b5 to 2c28d43 Compare January 10, 2025 22:43
@saponifi3d saponifi3d marked this pull request as ready for review January 10, 2025 23:05
@saponifi3d saponifi3d requested a review from a team as a code owner January 10, 2025 23:05


@condition_handler_registry.register(Condition.EVERY_EVENT)
class EveryEventConditionHandler(DataConditionHandler[WorkflowJob]):
type: DataConditionHandlerType = DataConditionHandlerType.WORKFLOW_TRIGGER
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cathteng can you confirm we need this condition handler? i think we might be able to remove it since it always evaluates as truthy (if we don't have a condition in a condition group that also evaluates as truthy).

Copy link
Member

Choose a reason for hiding this comment

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

i think you might be right, this was added in the past by default for issue alerts with no conditions in the WHEN 😬 but now we have a default evaluation for it

Copy link
Member

@cathteng cathteng left a comment

Choose a reason for hiding this comment

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

just these two should be workflow triggers!

Copy link

codecov bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/workflow_engine/types.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #83276       +/-   ##
===========================================
+ Coverage   46.63%   87.54%   +40.90%     
===========================================
  Files        9467     9481       +14     
  Lines      537059   537736      +677     
  Branches    21178    21178               
===========================================
+ Hits       250471   470757   +220286     
+ Misses     286238    66629   -219609     
  Partials      350      350               

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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