feat(migrations): Add in SafeDeleteModel migration operation

This adds in the `SafeDeleteModel` operation. Most of this pr is testing and monkeypatching Django migrations to be able to hook this in.

`SafeDeleteModel` allows us to delete models in two steps without using custom sql. If accepts a deletion_action parameter, which must be passed, and can be either MOVE_TO_PENDING or DELETE.

MOVE_TO_PENDING checks that there are no constraints on the table and removes the table from the migation state. We keep track of pending models separately, so that they can be deleted in the next step.

DELETE runs the delete command on the database. It is able to still reference the model since we keep track of this in our patched state. If delete is run before MOVE_TO_PENDING then the migration will be rejected.

Some care is still needed when using these - we still have to ensure that the PENDING migration runs and fully deploys before the DELETE does. But other than that this reduces the manual error prone sql work around this process a lot.

Author: wedamija

fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/__init__.py

@@ -0,0 +1,26 @@
+# Generated by Django 3.1 on 2019-09-22 21:47
+
+from django.db import migrations, models
+
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+
+    initial = True
+
+    dependencies = []
+
+    operations = [
+        migrations.CreateModel(
+            name="TestTable",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+                    ),
+                ),
+            ],
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0001_initial.py

@@ -0,0 +1,17 @@
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.new_migrations.monkey.models import SafeDeleteModel
+from sentry.new_migrations.monkey.state import DeletionAction
+
+
+class Migration(CheckedMigration):
+
+    dependencies = [
+        ("bad_flow_delete_model_double_pending_app", "0001_initial"),
+    ]
+
+    operations = [
+        SafeDeleteModel(
+            name="TestTable",
+            deletion_action=DeletionAction.MOVE_TO_PENDING,
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0002_delete_pending.py

@@ -0,0 +1,17 @@
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.new_migrations.monkey.models import SafeDeleteModel
+from sentry.new_migrations.monkey.state import DeletionAction
+
+
+class Migration(CheckedMigration):
+
+    dependencies = [
+        ("bad_flow_delete_model_double_pending_app", "0002_delete_pending"),
+    ]
+
+    operations = [
+        SafeDeleteModel(
+            name="TestTable",
+            deletion_action=DeletionAction.MOVE_TO_PENDING,
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/0003_double_pending.py

@@ -0,0 +1,5 @@
+from django.db import models
+
+
+class TestTable(models.Model):
+    field = models.IntegerField(default=0, null=False)

fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/migrations/__init__.py

@@ -0,0 +1,26 @@
+# Generated by Django 3.1 on 2019-09-22 21:47
+
+from django.db import migrations, models
+
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+
+    initial = True
+
+    dependencies = []
+
+    operations = [
+        migrations.CreateModel(
+            name="TestTable",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+                    ),
+                ),
+            ],
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_model_double_pending_app/models.py

@@ -0,0 +1,17 @@
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.new_migrations.monkey.models import SafeDeleteModel
+from sentry.new_migrations.monkey.state import DeletionAction
+
+
+class Migration(CheckedMigration):
+
+    dependencies = [
+        ("bad_flow_delete_model_without_pending_app", "0001_initial"),
+    ]
+
+    operations = [
+        SafeDeleteModel(
+            name="TestTable",
+            deletion_action=DeletionAction.DELETE,
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/__init__.py

@@ -0,0 +1,5 @@
+from django.db import models
+
+
+class TestTable(models.Model):
+    field = models.IntegerField(default=0, null=False)

fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/0001_initial.py

@@ -0,0 +1,45 @@
+import django
+from django.db import migrations, models
+
+import sentry
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+
+    initial = True
+
+    dependencies = []
+
+    operations = [
+        migrations.CreateModel(
+            name="FkTable",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+                    ),
+                ),
+            ],
+        ),
+        migrations.CreateModel(
+            name="TestTable",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+                    ),
+                ),
+                (
+                    "fk_table",
+                    sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        to="bad_flow_delete_pending_with_fk_constraints_app.fktable",
+                        db_index=False,
+                    ),
+                ),
+            ],
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/0002_delete_without_pending.py

@@ -0,0 +1,18 @@
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.new_migrations.monkey.models import SafeDeleteModel
+from sentry.new_migrations.monkey.state import DeletionAction
+
+
+class Migration(CheckedMigration):
+    atomic = False
+
+    dependencies = [
+        ("bad_flow_delete_pending_with_fk_constraints_app", "0001_initial"),
+    ]
+
+    operations = [
+        SafeDeleteModel(
+            name="TestTable",
+            deletion_action=DeletionAction.MOVE_TO_PENDING,
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/migrations/__init__.py

@@ -0,0 +1,12 @@
+from django.db import models
+
+from sentry.db.models import FlexibleForeignKey
+
+
+class FkTable(models.Model):
+    field = models.IntegerField(default=0, null=False)
+
+
+class TestTable(models.Model):
+    field = models.IntegerField(default=0, null=False)
+    fk_table = FlexibleForeignKey(FkTable, db_index=False)

fixtures/safe_migrations_apps/bad_flow_delete_model_without_pending_app/models.py

@@ -0,0 +1,46 @@
+# Generated by Django 3.1 on 2019-09-22 21:47
+import django
+from django.db import migrations, models
+
+import sentry
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+
+    initial = True
+
+    dependencies = []
+
+    operations = [
+        migrations.CreateModel(
+            name="FkTable",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+                    ),
+                ),
+            ],
+        ),
+        migrations.CreateModel(
+            name="TestTable",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+                    ),
+                ),
+                (
+                    "fk_table",
+                    sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        on_delete=django.db.models.deletion.CASCADE,
+                        to="good_flow_delete_pending_with_fk_constraints_app.fktable",
+                        db_index=False,
+                    ),
+                ),
+            ],
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/__init__.py

@@ -0,0 +1,32 @@
+import django
+from django.db import migrations
+
+import sentry
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.new_migrations.monkey.models import SafeDeleteModel
+from sentry.new_migrations.monkey.state import DeletionAction
+
+
+class Migration(CheckedMigration):
+    atomic = False
+
+    dependencies = [
+        ("good_flow_delete_pending_with_fk_constraints_app", "0001_initial"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="TestTable",
+            name="fk_table",
+            field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                to="good_flow_delete_pending_with_fk_constraints_app.fktable",
+                db_index=False,
+                db_constraint=False,
+            ),
+        ),
+        SafeDeleteModel(
+            name="TestTable",
+            deletion_action=DeletionAction.MOVE_TO_PENDING,
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/0001_initial.py

@@ -0,0 +1,16 @@
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.new_migrations.monkey.models import SafeDeleteModel
+from sentry.new_migrations.monkey.state import DeletionAction
+
+
+class Migration(CheckedMigration):
+    dependencies = [
+        ("good_flow_delete_pending_with_fk_constraints_app", "0002_remove_constraints_and_pending"),
+    ]
+
+    operations = [
+        SafeDeleteModel(
+            name="TestTable",
+            deletion_action=DeletionAction.DELETE,
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/0002_delete_without_pending.py

@@ -0,0 +1,12 @@
+from django.db import models
+
+from sentry.db.models import FlexibleForeignKey
+
+
+class FkTable(models.Model):
+    field = models.IntegerField(default=0, null=False)
+
+
+class TestTable(models.Model):
+    field = models.IntegerField(default=0, null=False)
+    fk_table = FlexibleForeignKey(FkTable, db_index=False)

fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/migrations/__init__.py

@@ -0,0 +1,22 @@
+from django.db import migrations, models
+
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    initial = True
+    dependencies = []
+
+    operations = [
+        migrations.CreateModel(
+            name="TestTable",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+                    ),
+                ),
+            ],
+        ),
+    ]

fixtures/safe_migrations_apps/bad_flow_delete_pending_with_fk_constraints_app/models.py

@@ -0,0 +1,16 @@
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.new_migrations.monkey.models import SafeDeleteModel
+from sentry.new_migrations.monkey.state import DeletionAction
+
+
+class Migration(CheckedMigration):
+    dependencies = [
+        ("good_flow_delete_simple_app", "0001_initial"),
+    ]
+
+    operations = [
+        SafeDeleteModel(
+            name="TestTable",
+            deletion_action=DeletionAction.MOVE_TO_PENDING,
+        ),
+    ]

fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/__init__.py

@@ -0,0 +1,16 @@
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.new_migrations.monkey.models import SafeDeleteModel
+from sentry.new_migrations.monkey.state import DeletionAction
+
+
+class Migration(CheckedMigration):
+    dependencies = [
+        ("good_flow_delete_simple_app", "0002_set_pending"),
+    ]
+
+    operations = [
+        SafeDeleteModel(
+            name="TestTable",
+            deletion_action=DeletionAction.DELETE,
+        ),
+    ]

fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0001_initial.py

@@ -0,0 +1,5 @@
+from django.db import models
+
+
+class TestTable(models.Model):
+    field = models.IntegerField(default=0, null=False)

fixtures/safe_migrations_apps/good_flow_delete_pending_with_fk_constraints_app/migrations/0002_remove_constraints_and_pending.py

@@ -88,14 +88,16 @@ def alter_db_table(self, model, old_db_table, new_db_table):
             "More info here: https://develop.sentry.dev/database-migrations/#renaming-tables"
         )
 
-    def delete_model(self, model):
+    def delete_model(self, model, is_safe=False):
         """
         It's never safe to delete a model using the standard migration process
         """
-        raise UnsafeOperationException(
-            f"Deleting the {model.__name__} model is unsafe.\n"
-            "More info here: https://develop.sentry.dev/database-migrations/#deleting-tables"
-        )
+        if not is_safe:
+            raise UnsafeOperationException(
+                f"Deleting the {model.__name__} model is unsafe.\n"
+                "More info here: https://develop.sentry.dev/database-migrations/#deleting-tables"
+            )
+        super(DatabaseSchemaEditorMixin, self).delete_model(model)
 
     def remove_field(self, model, field):
         """