feat(uptime): Switch `RemoteSubscription` to be an abstract base model. (#73113)

After working with `RemoteSubscription` for a little while it doesn't
feel very useful to have it as a whole separate table, and might cause
problems in the future due to shared data. It seems more useful as a
separate base class, and each subscription type just inherits from it
and gets a separate table.

<!-- Describe your PR here. -->

fixtures/backup/model_dependencies/detailed.json

@@ -261,22 +261,6 @@
     "table_name": "nodestore_node",
     "uniques": []
   },
-  "remote_subscriptions.remotesubscription": {
-    "dangling": false,
-    "foreign_keys": {},
-    "model": "remote_subscriptions.remotesubscription",
-    "relocation_dependencies": [],
-    "relocation_scope": "Excluded",
-    "silos": [
-      "Region"
-    ],
-    "table_name": "sentry_remotesubscription",
-    "uniques": [
-      [
-        "subscription_id"
-      ]
-    ]
-  },
   "replays.replayrecordingsegment": {
     "dangling": false,
     "foreign_keys": {
@@ -6279,13 +6263,7 @@
   },
   "uptime.uptimesubscription": {
     "dangling": false,
-    "foreign_keys": {
-      "remote_subscription": {
-        "kind": "FlexibleForeignKey",
-        "model": "remote_subscriptions.remotesubscription",
-        "nullable": false
-      }
-    },
+    "foreign_keys": {},
     "model": "uptime.uptimesubscription",
     "relocation_dependencies": [],
     "relocation_scope": "Excluded",
@@ -6299,7 +6277,7 @@
         "url"
       ],
       [
-        "remote_subscription"
+        "subscription_id"
       ]
     ]
   }

fixtures/backup/model_dependencies/flat.json

@@ -35,7 +35,6 @@
   "hybridcloud.regioncacheversion": [],
   "hybridcloud.webhookpayload": [],
   "nodestore.node": [],
-  "remote_subscriptions.remotesubscription": [],
   "replays.replayrecordingsegment": [
     "sentry.file",
     "sentry.project"
@@ -862,7 +861,5 @@
     "sentry.project",
     "uptime.uptimesubscription"
   ],
-  "uptime.uptimesubscription": [
-    "remote_subscriptions.remotesubscription"
-  ]
+  "uptime.uptimesubscription": []
 }

fixtures/backup/model_dependencies/sorted.json

@@ -3,7 +3,6 @@
   "hybridcloud.regioncacheversion",
   "hybridcloud.webhookpayload",
   "nodestore.node",
-  "remote_subscriptions.remotesubscription",
   "sentry.broadcast",
   "sentry.controlfile",
   "sentry.controlfileblob",

fixtures/backup/model_dependencies/truncate.json

@@ -3,7 +3,6 @@
   "hybridcloud_regioncacheversion",
   "hybridcloud_webhookpayload",
   "nodestore_node",
-  "sentry_remotesubscription",
   "sentry_broadcast",
   "sentry_controlfile",
   "sentry_controlfileblob",

migrations_lockfile.txt

@@ -8,8 +8,8 @@ will then be regenerated, and you should be able to merge without conflicts.
 feedback: 0004_index_together
 hybridcloud: 0016_add_control_cacheversion
 nodestore: 0002_nodestore_no_dictfield
-remote_subscriptions: 0001_remote_subscription
+remote_subscriptions: 0002_remove_separate_remote_subscription
 replays: 0004_index_together
 sentry: 0731_add_insight_project_flags
 social_auth: 0002_default_auto_field
-uptime: 0001_uptime_subscriptions
+uptime: 0002_remove_separate_remote_subscription

src/sentry/remote_subscriptions/migrations/0002_remove_separate_remote_subscription.py

@@ -0,0 +1,36 @@
+# Generated by Django 5.0.6 on 2024-06-21 17:12
+
+from django.db import migrations
+
+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.
+    # 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 post deployment:
+    # - Large data migrations. Typically we want these to be run manually 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
+    #   run this outside deployments so that we don't block them. Note that while adding an index
+    #   is a schema change, it's completely safe to run the operation after the code has deployed.
+    # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
+
+    is_post_deployment = False
+
+    dependencies = [
+        ("remote_subscriptions", "0001_remote_subscription"),
+        ("uptime", "0002_remove_separate_remote_subscription"),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            state_operations=[
+                migrations.DeleteModel(
+                    name="RemoteSubscription",
+                ),
+            ]
+        )
+    ]

src/sentry/remote_subscriptions/models.py

@@ -1,20 +1,9 @@
-from datetime import timedelta
 from enum import Enum
-from typing import ClassVar, Self
 
 from django.db import models
 
-from sentry.backup.scopes import RelocationScope
-from sentry.db.models import DefaultFieldsModel, region_silo_model
-from sentry.db.models.manager.base import BaseManager
-
-
-@region_silo_model
-class RemoteSubscription(DefaultFieldsModel):
-    # TODO: This should be included in export/import, but right now it has no relation to
-    # any projects/orgs. Will fix this in a later pr
-    __relocation_scope__ = RelocationScope.Excluded
 
+class BaseRemoteSubscription(models.Model):
     class Status(Enum):
         ACTIVE = 0
         CREATING = 1
@@ -28,10 +17,5 @@ class RemoteSubscription(DefaultFieldsModel):
     status = models.SmallIntegerField(default=Status.ACTIVE.value, db_index=True)
     subscription_id = models.TextField(unique=True, null=True)
 
-    objects: ClassVar[BaseManager[Self]] = BaseManager(
-        cache_fields=["pk"], cache_ttl=int(timedelta(hours=1).total_seconds())
-    )
-
     class Meta:
-        app_label = "remote_subscriptions"
-        db_table = "sentry_remotesubscription"
+        abstract = True

src/sentry/uptime/migrations/0002_remove_separate_remote_subscription.py

@@ -0,0 +1,85 @@
+# Generated by Django 5.0.6 on 2024-06-21 17:10
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+import sentry.db.models.fields.foreignkey
+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.
+    # 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 post deployment:
+    # - Large data migrations. Typically we want these to be run manually 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
+    #   run this outside deployments so that we don't block them. Note that while adding an index
+    #   is a schema change, it's completely safe to run the operation after the code has deployed.
+    # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
+
+    is_post_deployment = False
+
+    dependencies = [
+        ("uptime", "0001_uptime_subscriptions"),
+        ("remote_subscriptions", "0001_remote_subscription"),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            state_operations=[
+                migrations.RemoveField(
+                    model_name="uptimesubscription",
+                    name="remote_subscription",
+                ),
+                migrations.AddField(
+                    model_name="uptimesubscription",
+                    name="type",
+                    field=models.TextField(),
+                    preserve_default=False,
+                ),
+                migrations.AddField(
+                    model_name="uptimesubscription",
+                    name="status",
+                    field=models.SmallIntegerField(db_index=True, default=0),
+                ),
+            ],
+            database_operations=[
+                migrations.AlterField(
+                    model_name="uptimesubscription",
+                    name="remote_subscription",
+                    field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        null=True,
+                        on_delete=django.db.models.deletion.CASCADE,
+                        to="remote_subscriptions.remotesubscription",
+                        unique=True,
+                    ),
+                ),
+                migrations.RunSQL(
+                    """
+                    ALTER TABLE "uptime_uptimesubscription" ADD COLUMN "type" text NOT NULL;
+                    """,
+                    reverse_sql="""
+                        ALTER TABLE "uptime_uptimesubscription" DROP COLUMN "type";
+                        """,
+                    hints={"tables": ["uptime_uptimesubscription"]},
+                ),
+                migrations.RunSQL(
+                    """
+                    ALTER TABLE "uptime_uptimesubscription" ADD COLUMN "status" smallint NOT NULL DEFAULT 0;
+                    """,
+                    reverse_sql="""
+                        ALTER TABLE "uptime_uptimesubscription" DROP COLUMN "status";
+                        """,
+                    hints={"tables": ["uptime_uptimesubscription"]},
+                ),
+            ],
+        ),
+        migrations.AddField(
+            model_name="uptimesubscription",
+            name="subscription_id",
+            field=models.TextField(null=True, unique=True),
+        ),
+    ]

src/sentry/uptime/models.py

@@ -6,15 +6,15 @@ from django.db import models
 from sentry.backup.scopes import RelocationScope
 from sentry.db.models import DefaultFieldsModel, FlexibleForeignKey, region_silo_model
 from sentry.db.models.manager.base import BaseManager
+from sentry.remote_subscriptions.models import BaseRemoteSubscription
 
 
 @region_silo_model
-class UptimeSubscription(DefaultFieldsModel):
+class UptimeSubscription(BaseRemoteSubscription, DefaultFieldsModel):
     # TODO: This should be included in export/import, but right now it has no relation to
     # any projects/orgs. Will fix this in a later pr
     __relocation_scope__ = RelocationScope.Excluded
 
-    remote_subscription = FlexibleForeignKey("remote_subscriptions.RemoteSubscription", unique=True)
     # The url to check
     url = models.CharField(max_length=255)
     # How frequently to run the check in seconds
@@ -23,7 +23,7 @@ class UptimeSubscription(DefaultFieldsModel):
     timeout_ms = models.IntegerField()
 
     objects: ClassVar[BaseManager[Self]] = BaseManager(
-        cache_fields=["pk", "remote_subscription_id"],
+        cache_fields=["pk", "subscription_id"],
         cache_ttl=int(timedelta(hours=1).total_seconds()),
     )
 

tests/sentry/backup/snapshots/test_comparators/test_default_comparators.pysnap

@@ -1,5 +1,5 @@
 ---
-created: '2024-06-20T22:24:39.811774+00:00'
+created: '2024-06-21T17:24:56.742786+00:00'
 creator: sentry
 source: tests/sentry/backup/test_comparators.py
 ---
@@ -63,10 +63,6 @@ source: tests/sentry/backup/test_comparators.py
   - class: ForeignKeyComparator
     fields: []
   model_name: nodestore.node
-- comparators:
-  - class: ForeignKeyComparator
-    fields: []
-  model_name: remote_subscriptions.remotesubscription
 - comparators:
   - class: ForeignKeyComparator
     fields:
@@ -1628,6 +1624,5 @@ source: tests/sentry/backup/test_comparators.py
   model_name: uptime.projectuptimesubscription
 - comparators:
   - class: ForeignKeyComparator
-    fields:
-    - remote_subscription
+    fields: []
   model_name: uptime.uptimesubscription