Skip to content

feat(workflow-engine): Add TypedDicts for WorkflowValidator input format#110692

Merged
kcons merged 2 commits intomasterfrom
kcons/typeytime
Mar 16, 2026
Merged

feat(workflow-engine): Add TypedDicts for WorkflowValidator input format#110692
kcons merged 2 commits intomasterfrom
kcons/typeytime

Conversation

@kcons
Copy link
Member

@kcons kcons commented Mar 13, 2026

Allows us to lean on mypy a bit more for correctness.

@kcons kcons requested review from a team as code owners March 13, 2026 22:12
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 13, 2026
@kcons kcons requested a review from ceorourke March 13, 2026 22:17
Copy link
Contributor

@saponifi3d saponifi3d left a comment

Choose a reason for hiding this comment

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

🫶 - love the addition of types and stricter checking than before.

"name": data.get("name"),
) -> WorkflowInput:
workflow_payload: WorkflowInput = {
"name": data.get("name", ""),
Copy link
Contributor

Choose a reason for hiding this comment

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

we might want to have the name error if it's an empty string. i believe there's a unique constraint on the names, so it could get a little 😵

triggers: dict[str, Any] = {"logicType": "any-short", "conditions": []}
translated_filter_list = []
triggers: DataConditionGroupInput = {"logicType": "any-short", "conditions": []}
translated_filter_list: list[DataConditionInput] = []
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 should we have an DetectorTrigger, ActionFilterInput, WorkflowTriggerInput as detailed types? that way we can enforce stuff like a detectors conditions always must return a valid PriorityLevel? (not sure if there are other things we could enforce more strictly down, i feel like those might be helpful for folks making custom validators like the metric alert validator)

@kcons kcons force-pushed the kcons/typeytime branch from ed09cf3 to d21a902 Compare March 16, 2026 21:19
Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

@kcons kcons marked this pull request as draft March 16, 2026 21:25
@github-actions
Copy link
Contributor

github-actions bot commented Mar 16, 2026

Backend Test Failures

Failures on 289098c in this run:

tests/sentry/api/endpoints/test_project_rule_details.py::UpdateProjectRuleTest::test_invalid_rule_nodelog
tests/sentry/api/endpoints/test_project_rule_details.py:1451: in test_invalid_rule_node
    self.get_error_response(
src/sentry/testutils/cases.py:663: in get_error_response
    assert_status_code(response, status_code)
src/sentry/testutils/asserts.py:47: in assert_status_code
    assert minimum <= response.status_code < maximum, (
E   AssertionError: (500, b'{"detail":"Internal Error","errorId":null}')
E   assert 500 < 401
E    +  where 500 = <Response status_code=500, "application/json">.status_code
tests/sentry/api/endpoints/test_project_rule_details.py::UpdateProjectRuleTest::test_rule_form_not_validlog
tests/sentry/api/endpoints/test_project_rule_details.py:1471: in test_rule_form_not_valid
    self.get_error_response(
src/sentry/testutils/cases.py:663: in get_error_response
    assert_status_code(response, status_code)
src/sentry/testutils/asserts.py:47: in assert_status_code
    assert minimum <= response.status_code < maximum, (
E   AssertionError: (500, b'{"detail":"Internal Error","errorId":null}')
E   assert 500 < 401
E    +  where 500 = <Response status_code=500, "application/json">.status_code
tests/sentry/api/endpoints/test_project_rule_details.py::UpdateProjectRuleTest::test_invalid_rule_node_typelog
tests/sentry/api/endpoints/test_project_rule_details.py:1431: in test_invalid_rule_node_type
    self.get_error_response(
src/sentry/testutils/cases.py:663: in get_error_response
    assert_status_code(response, status_code)
src/sentry/testutils/asserts.py:47: in assert_status_code
    assert minimum <= response.status_code < maximum, (
E   AssertionError: (500, b'{"detail":"Internal Error","errorId":null}')
E   assert 500 < 401
E    +  where 500 = <Response status_code=500, "application/json">.status_code

@kcons kcons force-pushed the kcons/typeytime branch from 8f11d0a to 3c6e7d0 Compare March 16, 2026 21:38
@kcons kcons marked this pull request as ready for review March 16, 2026 22:12
@kcons kcons merged commit 8a4b276 into master Mar 16, 2026
65 checks passed
@kcons kcons deleted the kcons/typeytime branch March 16, 2026 22:45
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.

3 participants