Skip to content

ref(workflow_engine): Convert OwnerModel.owner_team FK to BigInt#109938

Merged
beezz merged 4 commits intomasterfrom
beezz/ref/owner-team-break-fk-constraint
Mar 6, 2026
Merged

ref(workflow_engine): Convert OwnerModel.owner_team FK to BigInt#109938
beezz merged 4 commits intomasterfrom
beezz/ref/owner-team-break-fk-constraint

Conversation

@beezz
Copy link
Copy Markdown
Contributor

@beezz beezz commented Mar 5, 2026

The OwnerModel abstract base class defines owner_team as a FlexibleForeignKey
to sentry.Team. This FK creates a database-level constraint that assumes both
tables reside in the same database, which is not desirable in our sharded
environment where tables may be placed on different database instances.

This converts owner_team to a BoundedBigIntegerField (owner_team_id) for
the two concrete models that inherit from OwnerModel: Detector and Workflow
(both in workflow_engine app). The Rule model defines its own owner_team FK
separately and is not in scope.

The migration uses SeparateDatabaseAndState to drop the FK constraint at the
database level while updating Django state to use a plain integer field. Also adds
nullification of owner_team_id on Detector and Workflow in the team deletion
handler, matching the existing pattern for Rule and Monitor.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 5, 2026
Remove the database-level FK constraint on owner_team by converting
it from FlexibleForeignKey to BoundedBigIntegerField. This affects
Detector and Workflow models (both in workflow_engine app).

The migration uses SeparateDatabaseAndState to drop the FK constraint
at the database level while updating Django state to use a plain
integer field. Also adds nullification of owner_team_id on Detector
and Workflow when a Team is deleted.

Co-Authored-By: Claude <noreply@anthropic.com>
@beezz beezz force-pushed the beezz/ref/owner-team-break-fk-constraint branch from 72c285b to 71e3699 Compare March 5, 2026 13:24
@beezz beezz marked this pull request as ready for review March 5, 2026 13:24
@beezz beezz requested review from a team as code owners March 5, 2026 13:24
Copy link
Copy Markdown
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.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Migration removes non-existent constraint from Workflow state
    • Removed the Workflow RemoveConstraint and corresponding AddConstraint state operations in migration 0110 because that constraint never existed in workflow migration state or model metadata.

Create PR

Or push these changes by commenting:

@cursor push 8372276a40
Preview (8372276a40)
diff --git a/src/sentry/workflow_engine/migrations/0110_owner_team_break_fk.py b/src/sentry/workflow_engine/migrations/0110_owner_team_break_fk.py
--- a/src/sentry/workflow_engine/migrations/0110_owner_team_break_fk.py
+++ b/src/sentry/workflow_engine/migrations/0110_owner_team_break_fk.py
@@ -72,10 +72,6 @@
                 ),
             ],
             state_operations=[
-                migrations.RemoveConstraint(
-                    model_name="workflow",
-                    name="workflow_engine_workflow_owner_constraints",
-                ),
                 migrations.RemoveField(
                     model_name="workflow",
                     name="owner_team",
@@ -87,17 +83,6 @@
                         blank=True, db_index=True, null=True
                     ),
                 ),
-                migrations.AddConstraint(
-                    model_name="workflow",
-                    constraint=models.CheckConstraint(
-                        condition=(
-                            models.Q(owner_user_id__isnull=True, owner_team_id__isnull=False)
-                            | models.Q(owner_user_id__isnull=False, owner_team_id__isnull=True)
-                            | models.Q(owner_user_id__isnull=True, owner_team_id__isnull=True)
-                        ),
-                        name="workflow_engine_workflow_owner_constraints",
-                    ),
-                ),
             ],
         ),
     ]
This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 5, 2026

This PR has a migration; here is the generated SQL for src/sentry/workflow_engine/migrations/0110_owner_team_break_fk.py

for 0110_owner_team_break_fk in workflow_engine

--
-- Custom state/database change combination
--
SET CONSTRAINTS "workflow_engine_dete_owner_team_id_177670f3_fk_sentry_te" IMMEDIATE; ALTER TABLE "workflow_engine_detector" DROP CONSTRAINT "workflow_engine_dete_owner_team_id_177670f3_fk_sentry_te";
--
-- Custom state/database change combination
--
SET CONSTRAINTS "workflow_engine_work_owner_team_id_c401e7b8_fk_sentry_te" IMMEDIATE; ALTER TABLE "workflow_engine_workflow" DROP CONSTRAINT "workflow_engine_work_owner_team_id_c401e7b8_fk_sentry_te";

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 5, 2026

Backend Test Failures

Failures on 0e25ffd in this run:

tests/sentry/api/serializers/test_rule.py::WorkflowRuleSerializerTest::test_fetch_workflow_owner__teamlog
tests/sentry/api/serializers/test_rule.py:282: in test_fetch_workflow_owner__team
    workflow = self.create_workflow(owner_team=team)
src/sentry/testutils/fixtures.py:690: in create_workflow
    return Factories.create_workflow(*args, **kwargs)
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/contextlib.py:85: in inner
    return func(*args, **kwds)
src/sentry/testutils/factories.py:2269: in create_workflow
    return Workflow.objects.create(
src/sentry/silo/base.py:158: in override
    return original_method(*args, **kwargs)
.venv/lib/python3.13/site-packages/django/db/models/manager.py:87: in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
src/sentry/silo/base.py:158: in override
    return original_method(*args, **kwargs)
.venv/lib/python3.13/site-packages/django/db/models/query.py:663: in create
    obj = self.model(**kwargs)
.venv/lib/python3.13/site-packages/django/db/models/base.py:569: in __init__
    raise TypeError(
E   TypeError: Workflow() got unexpected keyword arguments: 'owner_team'

Workflow.Meta overrides OwnerModel.Meta.constraints with its own list,
so the CheckConstraint was never applied to Workflow. Remove the
RemoveConstraint/AddConstraint operations from the migration for
Workflow. Also fix test using owner_team= kwarg.

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Looks good to me. The constraint drops will leave indexes behind so query performance should not be impacted.

Comment on lines +32 to +33
Detector.objects.filter(owner_team_id=instance.id).update(owner_team_id=None)
Workflow.objects.filter(owner_team_id=instance.id).update(owner_team_id=None)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like most team deletions are done through scheduled deletions so we shouldn't have dangling references in detectors/workflows.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed that scheduled deletions handle the cascading cleanup when the FK exists. However, this PR converts owner_team from a ForeignKey (with on_delete=SET_NULL) to a plain BoundedBigIntegerField, so the DB will no longer automatically null these out on team deletion. The manual cleanup here ensures we don't end up with dangling team IDs pointing to deleted teams.

— Claude Code

Copy link
Copy Markdown
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.

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

Rule.objects.filter(owner_team_id=instance.id).update(owner_team_id=None)
Monitor.objects.filter(owner_team_id=instance.id).update(owner_team_id=None)
Detector.objects.filter(owner_team_id=instance.id).update(owner_team_id=None)
Workflow.objects.filter(owner_team_id=instance.id).update(owner_team_id=None)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Custom managers skip nullifying deletion-state objects' team references

Low Severity

Detector.objects and Workflow.objects use custom managers (DetectorManager, WorkflowManager) that exclude rows with PENDING_DELETION or DELETION_IN_PROGRESS status. These nullification queries will silently skip any detectors or workflows in those states, leaving dangling owner_team_id values. Previously, the database-level ON DELETE SET_NULL from the FK constraint would have nullified all rows regardless of Django manager filtering. This is unlikely to cause real problems since those objects are mid-deletion, but it's a subtle behavioral change from the FK-based cascade.

Fix in Cursor Fix in Web

@beezz beezz merged commit c2b0499 into master Mar 6, 2026
77 of 78 checks passed
@beezz beezz deleted the beezz/ref/owner-team-break-fk-constraint branch March 6, 2026 10:31
@github-actions github-actions bot locked and limited conversation to collaborators Mar 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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