feat(workflow_engine): Add a DataConditionHandlerType to each DataConditionHandler
#83276
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
Scope: Backend
…he handlers in APIs, allowing us to generate dropdowns for each product area (detectors, workflows, actions)
saponifi3d
force-pushed
the
jcallender/aci/data-conditions-api-spike
branch
2 times, most recently
from
c2d790a to
f39a0b5
Compare
…will allow us to have filtered dropdowns in the ui
saponifi3d
force-pushed
the
jcallender/aci/data-conditions-api-spike
branch
from
f39a0b5 to
2c28d43
Compare
saponifi3d
commented
Jan 10, 2025
|
|
||
|
|
||
| @condition_handler_registry.register(Condition.EVERY_EVENT) | ||
| class EveryEventConditionHandler(DataConditionHandler[WorkflowJob]): | ||
| type: DataConditionHandlerType = DataConditionHandlerType.WORKFLOW_TRIGGER |
There was a problem hiding this comment.
@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).
There was a problem hiding this comment.
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
cathteng
reviewed
Jan 10, 2025
src/sentry/workflow_engine/handlers/condition/existing_high_priority_issue_handler.py
Outdated
Show resolved
Hide resolved
src/sentry/workflow_engine/handlers/condition/reappeared_event_handler.py
Outdated
Show resolved
Hide resolved
…_handler.py Co-authored-by: Cathy Teng <70817427+cathteng@users.noreply.github.com>
…iority_issue_handler.py Co-authored-by: Cathy Teng <70817427+cathteng@users.noreply.github.com>
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 #83276 +/- ##
==========================================
+ Coverage 87.53% 87.54% +0.01%
==========================================
Files 9405 9409 +4
Lines 537590 537741 +151
Branches 21175 21120 -55
==========================================
+ Hits 470574 470770 +196
+ Misses 66668 66623 -45
Partials 348 348 |
cathteng
approved these changes
Jan 14, 2025
src/sentry/workflow_engine/handlers/condition/issue_occurrences_handler.py
Outdated
Show resolved
Hide resolved
cathteng
reviewed
Jan 14, 2025
src/sentry/workflow_engine/types.py
Outdated
| @@ -55,6 +62,10 @@ def bulk_get_query_object(data_sources) -> dict[int, T | None]: | |||
|
|
|||
|
|
|||
| class DataConditionHandler(Generic[T]): | |||
| @abstractproperty | |||
| def type(self) -> DataConditionHandlerType: | |||
There was a problem hiding this comment.
is it weird that to use this type, we'll have to initialize the handler? i'm finding this with using abstract properties for enforcing data conditions json schemas
There was a problem hiding this comment.
updated to use the ClassVar - it's nice that i can access it statically, but yeah, the typing isn't great or there needs to be a default value. I'll stick with ClassVar + default for now for consistency and cause it has the best general mixture of type safety and customization.
…s_handler.py Co-authored-by: Cathy Teng <70817427+cathteng@users.noreply.github.com>
andrewshie-sentry
pushed a commit
that referenced
this pull request
Jan 22, 2025
…onditionHandler (#83276) ## 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) --------- Co-authored-by: Cathy Teng <70817427+cathteng@users.noreply.github.com> Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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;