feat(ACI): Use status column on Detector and Workflow #91994

ceorourke · 2025-05-20T23:18:28Z

Now that the Detector and Workflow models have a status column we can make use of it. This PR sets the status to PENDING_DELETION in the deletion endpoints, sets the status to DELETION_IN_PROGRESS in the deletion tasks, and updates all gets and filters to filter to ACTIVE statuses.

codecov · 2025-05-20T23:44:41Z

⚠️ Parser warning

The parser emitted a warning. Please review your JUnit XML file:

Warning while parsing testcase attributes: Limit of string is 1000 chars, for name, we got 2083 at 1:156310 in /home/runner/work/sentry/sentry/.artifacts/pytest.junit.xml

❌ 8 Tests Failed:

Tests completed	Failed	Passed	Skipped
25952	8	25944	206

View the top 3 failed test(s) by shortest run time

tests.sentry.workflow_engine.endpoints.test_organization_workflow_details.OrganizationDeleteWorkflowTest::test_delete_configured_workflow__action

Stack Traces | 2.28s run time

#x1B[1m#x1B[.../workflow_engine/endpoints/test_organization_workflow_details.py#x1B[0m:136: in test_delete_configured_workflow__action
    assert not Action.objects.filter(id=action.id).exists()
#x1B[1m#x1B[31mE   AssertionError: assert not True#x1B[0m
#x1B[1m#x1B[31mE    +  where True = <bound method QuerySet.exists of <BaseQuerySet [<Action at 0x7f4600833770: id=16, type='slack'>]>>()#x1B[0m
#x1B[1m#x1B[31mE    +    where <bound method QuerySet.exists of <BaseQuerySet [<Action at 0x7f4600833770: id=16, type='slack'>]>> = <BaseQuerySet [<Action at 0x7f4600833770: id=16, type='slack'>]>.exists#x1B[0m
#x1B[1m#x1B[31mE    +      where <BaseQuerySet [<Action at 0x7f4600833770: id=16, type='slack'>]> = <bound method QuerySet.filter of <sentry.db.models.manager.base.BaseManager object at 0x7f4675332900>>(id=16)#x1B[0m
#x1B[1m#x1B[31mE    +        where <bound method QuerySet.filter of <sentry.db.models.manager.base.BaseManager object at 0x7f4675332900>> = <sentry.db.models.manager.base.BaseManager object at 0x7f4675332900>.filter#x1B[0m
#x1B[1m#x1B[31mE    +          where <sentry.db.models.manager.base.BaseManager object at 0x7f4675332900> = Action.objects#x1B[0m
#x1B[1m#x1B[31mE    +        and   16 = <Action at 0x7f46008331d0: id=16, type=<Type.SLACK: 'slack'>>.id#x1B[0m

tests.sentry.workflow_engine.endpoints.test_organization_workflow_details.OrganizationDeleteWorkflowTest::test_delete_configured_workflow__action_condition

Stack Traces | 2.48s run time

#x1B[1m#x1B[.../workflow_engine/endpoints/test_organization_workflow_details.py#x1B[0m:154: in test_delete_configured_workflow__action_condition
    assert not DataConditionGroup.objects.filter(id=action_condition_group.id).exists()
#x1B[1m#x1B[31mE   AssertionError: assert not True#x1B[0m
#x1B[1m#x1B[31mE    +  where True = <bound method QuerySet.exists of <BaseQuerySet [<DataConditionGroup at 0x7f4246ab50d0: id=20, logic_type='any-short'>]>>()#x1B[0m
#x1B[1m#x1B[31mE    +    where <bound method QuerySet.exists of <BaseQuerySet [<DataConditionGroup at 0x7f4246ab50d0: id=20, logic_type='any-short'>]>> = <BaseQuerySet [<DataConditionGroup at 0x7f42446105d0: id=20, logic_type='any-short'>]>.exists#x1B[0m
#x1B[1m#x1B[31mE    +      where <BaseQuerySet [<DataConditionGroup at 0x7f42446105d0: id=20, logic_type='any-short'>]> = <bound method QuerySet.filter of <sentry.db.models.manager.base.BaseManager object at 0x7f428ee97620>>(id=20)#x1B[0m
#x1B[1m#x1B[31mE    +        where <bound method QuerySet.filter of <sentry.db.models.manager.base.BaseManager object at 0x7f428ee97620>> = <sentry.db.models.manager.base.BaseManager object at 0x7f428ee97620>.filter#x1B[0m
#x1B[1m#x1B[31mE    +          where <sentry.db.models.manager.base.BaseManager object at 0x7f428ee97620> = DataConditionGroup.objects#x1B[0m
#x1B[1m#x1B[31mE    +        and   20 = <DataConditionGroup at 0x7f4244829950: id=20, logic_type='any-short'>.id#x1B[0m

tests.sentry.workflow_engine.endpoints.test_organization_detector_details.OrganizationDetectorDetailsGetTest::test_pending_deletion

Stack Traces | 2.78s run time

#x1B[1m#x1B[.../workflow_engine/endpoints/test_organization_detector_details.py#x1B[0m:98: in test_pending_deletion
    self.get_error_response(self.organization.slug, self.detector.id, status_code=404)
#x1B[1m#x1B[.../sentry/testutils/cases.py#x1B[0m:648: in get_error_response
    assert_status_code(response, status_code)
#x1B[1m#x1B[.../sentry/testutils/asserts.py#x1B[0m:45: in assert_status_code
    assert minimum <= response.status_code < maximum, (
#x1B[1m#x1B[31mE   AssertionError: (200, b'{"id":"21","projectId":"4556112054976514","name":"Test Detector","type":"metric_issue","workflowIds":[],"dateC...:"lt","comparison":50,"conditionResult":25}],"actions":[]},"config":{"detection_type":"static","threshold_period":1}}')#x1B[0m
#x1B[1m#x1B[31mE   assert 404 <= 200#x1B[0m
#x1B[1m#x1B[31mE    +  where 200 = <Response status_code=200, "application/json">.status_code#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

ceorourke · 2025-05-21T17:20:02Z

src/sentry/workflow_engine/endpoints/organization_workflow_details.py

@@ -107,6 +108,7 @@ def delete(self, request: Request, organization: Organization, workflow: Workflo
        Delete a workflow
        """
        RegionScheduledDeletion.schedule(workflow, days=0, actor=request.user)
+        workflow.update(status=ObjectStatus.PENDING_DELETION)


Setting it to pending deletion in the delete endpoint after it's been scheduled for deletion

ceorourke · 2025-05-21T17:21:07Z

src/sentry/workflow_engine/endpoints/organization_detector_details.py

@@ -169,6 +172,7 @@ def delete(self, request: Request, organization: Organization, detector: Detecto
            return Response(status=403)

        RegionScheduledDeletion.schedule(detector, days=0, actor=request.user)
+        detector.update(status=ObjectStatus.PENDING_DELETION)


Setting it to pending deletion in the delete endpoint after it's been scheduled for deletion

ceorourke · 2025-05-21T17:21:25Z

src/sentry/deletions/defaults/detector.py

+        for instance in instance_list:
+            status = getattr(instance, "status", None)
+            if status not in (ObjectStatus.DELETION_IN_PROGRESS, None):
+                instance.update(status=ObjectStatus.DELETION_IN_PROGRESS)


Setting it to deletion in progress

ceorourke · 2025-05-21T17:21:32Z

src/sentry/deletions/defaults/workflow.py

+        for instance in instance_list:
+            status = getattr(instance, "status", None)
+            if status not in (ObjectStatus.DELETION_IN_PROGRESS, None):
+                instance.update(status=ObjectStatus.DELETION_IN_PROGRESS)


Setting it to deletion in progress

ceorourke · 2025-05-21T17:23:01Z

I know it's a lot of files touched but it's the same change across most except those that I called out in the comments.

saponifi3d · 2025-05-21T23:36:41Z

src/sentry/incidents/endpoints/serializers/workflow_engine_data_condition.py

@@ -87,7 +89,9 @@ def serialize(
        **kwargs: Any,
    ) -> dict[str, Any]:
        # XXX: we are assuming that the obj/DataCondition is a detector trigger
-        detector = Detector.objects.get(workflow_condition_group=obj.condition_group)
+        detector = Detector.objects.get(


is there a way we can set status=ObjectStatus.ACTIVE as a default value when fetching then have the ability to override it? that way we can ensure it only selects active detectors unless we otherwise query.

maybe something like:

class DetectorManager(models.Manager): def filter(self, *args, **kwargs): # Only add the default status if no status is explicitly provided if 'status' not in kwargs: kwargs['status'] = ObjectStatus.ACTIVE return super().filter(*args, **kwargs) def get(self, *args, **kwargs): # Only add the default status if no status is explicitly provided if 'status' not in kwargs: kwargs['status'] = ObjectStatus.ACTIVE return super().get(*args, **kwargs) class Detector(models.Model): # Your fields here status = models.IntegerField(choices=ObjectStatus.choices, default=ObjectStatus.ACTIVE) objects = DetectorManager()

nvm blonde moment

ceorourke · 2025-05-22T00:30:45Z

src/sentry/workflow_engine/endpoints/validators/detector_workflow.py

@@ -50,7 +50,8 @@ def create(self, validated_data):
                if not can_edit_detector(detector, self.context["request"]):
                    raise PermissionDenied
                workflow = Workflow.objects.get(
-                    organization=self.context["organization"], id=validated_data["workflow_id"]
+                    organization=self.context["organization"],
+                    id=validated_data["workflow_id"],


I can't put this back so just ignore 😢

saponifi3d

🎉 thanks for doing that refactor - if you don't mind adding tests for the managers as well that'd be awesome, otherwise lgtm!

saponifi3d · 2025-05-22T16:48:02Z

src/sentry/workflow_engine/models/workflow.py

 from sentry.models.owner_base import OwnerModel
 from sentry.workflow_engine.models.data_condition import DataCondition, is_slow_condition
 from sentry.workflow_engine.types import WorkflowEventData

 from .json_config import JSONConfigBase


+class WorkflowManager(BaseManager["Workflow"]):


might be nice to have some individual tests for the managers to ensure it's working as expected.

ceorourke · 2025-05-22T17:03:53Z

uh ... oops

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 20, 2025

vercel bot deployed to Preview May 20, 2025 23:56 View deployment

ceorourke commented May 21, 2025

View reviewed changes

ceorourke marked this pull request as ready for review May 21, 2025 17:23

ceorourke requested review from a team as code owners May 21, 2025 17:23

saponifi3d reviewed May 21, 2025

View reviewed changes

vercel bot deployed to Preview May 22, 2025 00:27 View deployment

vercel bot deployed to Preview May 22, 2025 00:30 View deployment

ceorourke commented May 22, 2025

View reviewed changes

ceorourke requested a review from saponifi3d May 22, 2025 00:30

vercel bot deployed to Preview May 22, 2025 00:31 View deployment

saponifi3d approved these changes May 22, 2025

View reviewed changes

ceorourke closed this May 22, 2025

ceorourke force-pushed the ceorourke/ACI-286 branch from 9a4474d to a8a30ea Compare May 22, 2025 17:02

vercel bot deployed to Preview May 22, 2025 17:05 View deployment

ceorourke mentioned this pull request May 22, 2025

feat(ACI): Use status column on Detector and Workflow #92166

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(ACI): Use status column on Detector and Workflow #91994

feat(ACI): Use status column on Detector and Workflow #91994

Uh oh!

ceorourke commented May 20, 2025

Uh oh!

codecov bot commented May 20, 2025 •

edited

Loading

Uh oh!

ceorourke May 21, 2025

Uh oh!

ceorourke May 21, 2025

Uh oh!

ceorourke May 21, 2025

Uh oh!

ceorourke May 21, 2025

Uh oh!

ceorourke commented May 21, 2025

Uh oh!

saponifi3d May 21, 2025

Uh oh!

ceorourke May 21, 2025

Uh oh!

ceorourke May 22, 2025

Uh oh!

saponifi3d left a comment

Uh oh!

saponifi3d May 22, 2025

Uh oh!

ceorourke commented May 22, 2025

Uh oh!

Uh oh!

Uh oh!

feat(ACI): Use status column on Detector and Workflow #91994

feat(ACI): Use status column on Detector and Workflow #91994

Uh oh!

Conversation

ceorourke commented May 20, 2025

Uh oh!

codecov bot commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Parser warning

❌ 8 Tests Failed:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ceorourke commented May 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saponifi3d left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ceorourke commented May 22, 2025

Uh oh!

Uh oh!

codecov bot commented May 20, 2025 •

edited

Loading