Browse Source

ref(hybridcloud) Break User.actor foreign key (#49425)

Remove the constraint between user + actor. I've removed all the queries
using the relation in advance of this change. Next I'll remove the
writes to this field and finally remove the field.

Continuation of #49141
Mark Story 1 year ago
parent
commit
e13f21adef

+ 1 - 1
migrations_lockfile.txt

@@ -6,5 +6,5 @@ To resolve this, rebase against latest master and regenerate your migration. Thi
 will then be regenerated, and you should be able to merge without conflicts.
 
 nodestore: 0002_nodestore_no_dictfield
-sentry: 0451_pickle_to_json_sentry_controloption
+sentry: 0452_break_user_actor_fk
 social_auth: 0001_initial

+ 54 - 0
src/sentry/migrations/0452_break_user_actor_fk.py

@@ -0,0 +1,54 @@
+# Generated by Django 2.2.28 on 2023-05-18 15:28
+
+from django.db import migrations, models
+
+from sentry.db.models.fields.foreignkey import FlexibleForeignKey
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, this should only be used for operations where it's safe to run the migration
+    # after your code has deployed. So this should not be used for most operations that alter the
+    # schema of a table.
+    # Here are some things that make sense to mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that they can
+    #   be monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = False
+
+    dependencies = [
+        ("sentry", "0451_pickle_to_json_sentry_controloption"),
+    ]
+
+    operations = [
+        # Remove the foreign key on auth_user.actor_id
+        migrations.AlterField(
+            model_name="user",
+            name="actor",
+            field=FlexibleForeignKey(
+                to="sentry.Actor", db_constraint=False, db_index=True, unique=True, null=True
+            ),
+        ),
+        migrations.SeparateDatabaseAndState(
+            database_operations=[],
+            state_operations=[
+                migrations.AlterField(
+                    model_name="user",
+                    name="actor",
+                    field=models.BigIntegerField(
+                        null=True,
+                        db_index=True,
+                        unique=True,
+                    ),
+                ),
+                migrations.RenameField(
+                    model_name="user",
+                    old_name="actor",
+                    new_name="actor_id",
+                ),
+            ],
+        ),
+    ]

+ 1 - 1
src/sentry/models/actor.py

@@ -192,7 +192,7 @@ class ActorTuple(namedtuple("Actor", "id type")):
     def resolve_to_actor(self) -> Actor:
         obj = self.resolve()
         # TODO(actorid) Remove this once user no longer has actor_id.
-        if obj.actor_id is None or isinstance(obj, RpcUser):
+        if getattr(obj, "actor_id") is None or isinstance(obj, RpcUser):
             return get_actor_for_user(obj)
         # Team case
         return Actor.objects.get(id=obj.actor_id)

+ 2 - 5
src/sentry/models/user.py

@@ -19,7 +19,6 @@ from sentry.db.models import (
     BaseManager,
     BaseModel,
     BoundedAutoField,
-    FlexibleForeignKey,
     control_silo_only_model,
     sane_repr,
 )
@@ -187,14 +186,12 @@ class User(BaseModel, AbstractBaseUser):
     )
 
     session_nonce = models.CharField(max_length=12, null=True)
-    actor = FlexibleForeignKey(
-        "sentry.Actor",
-        related_name="user_from_actor",
+    actor_id = models.BigIntegerField(
         db_index=True,
         unique=True,
         null=True,
-        on_delete=models.PROTECT,
     )
+
     date_joined = models.DateTimeField(_("date joined"), default=timezone.now)
     last_active = models.DateTimeField(_("last active"), default=timezone.now, null=True)
 

+ 2 - 3
tests/sentry/api/endpoints/test_project_rule_details.py

@@ -18,9 +18,8 @@ from sentry.models import (
     RuleFireHistory,
     RuleSnooze,
     RuleStatus,
-    User,
 )
-from sentry.models.actor import get_actor_for_user
+from sentry.models.actor import Actor, get_actor_for_user
 from sentry.testutils import APITestCase
 from sentry.testutils.helpers import install_slack
 from sentry.testutils.silo import exempt_from_silo_limits, region_silo_test
@@ -37,7 +36,7 @@ def assert_rule_from_payload(rule: Rule, payload: Mapping[str, Any]) -> None:
     owner_id = payload.get("owner")
     if owner_id:
         with exempt_from_silo_limits():
-            assert rule.owner == User.objects.get(id=owner_id).actor
+            assert Actor.objects.get(id=rule.owner_id)
     else:
         assert rule.owner is None
 

+ 1 - 2
tests/sentry/db/test_silo_models.py

@@ -1,5 +1,5 @@
 from sentry.api.serializers.base import registry
-from sentry.models import Actor, OrganizationMember, User
+from sentry.models import OrganizationMember, User
 from sentry.testutils.silo import (
     validate_models_have_silos,
     validate_no_cross_silo_deletions,
@@ -9,7 +9,6 @@ from sentry.testutils.silo import (
 decorator_exemptions = set()
 fk_exemptions = {
     (OrganizationMember, User),
-    (User, Actor),
 }
 
 

+ 1 - 6
tests/sentry/models/test_actor.py

@@ -12,11 +12,6 @@ class ActorTest(TestCase):
         assert type(team.actor) is Actor
         assert team.actor.team_id == team.id
 
-        # TODO(hybridcloud) Remove when writes to User.actor_id are removed.
         actor = Actor.objects.create(type=1)
         user2 = User.objects.create(username="meow", actor_id=actor.id)
-        assert user2.actor == actor
-
-        actor = Actor.objects.create(type=1)
-        user3 = User.objects.create(username="woof", actor=actor)
-        assert user3.actor == actor
+        assert user2.actor_id == actor.id