Browse Source

ref(debugfile): Remove project fk from debugfile model (#27066)

* ref(debugfile): Remove project fk from debugfile model

* Fix debug_file api endpoint use of ProjectDebugFile.project_id

* Fix some more use of ProjectDebugFile.project_id

* Fix symbolicator tests

* Fix migrations_lockfile

* Remove commented line
Michal Kuffa 3 years ago
parent
commit
3d6d1a3a0f

+ 1 - 1
migrations_lockfile.txt

@@ -7,5 +7,5 @@ will then be regenerated, and you should be able to merge without conflicts.
 
 jira_ac: 0001_initial
 nodestore: 0002_nodestore_no_dictfield
-sentry: 0216_cdc_setup_replication_index
+sentry: 0217_debugfile_remove_project_fk
 social_auth: 0001_initial

+ 3 - 3
src/sentry/api/endpoints/debug_files.py

@@ -177,7 +177,7 @@ class DebugFilesEndpoint(ProjectEndpoint):
 
         q &= file_format_q
 
-        queryset = ProjectDebugFile.objects.filter(q, project=project).select_related("file")
+        queryset = ProjectDebugFile.objects.filter(q, project_id=project.id).select_related("file")
 
         return self.paginate(
             request=request,
@@ -206,7 +206,7 @@ class DebugFilesEndpoint(ProjectEndpoint):
         if request.GET.get("id") and (request.access.has_scope("project:write")):
             with transaction.atomic():
                 debug_file = (
-                    ProjectDebugFile.objects.filter(id=request.GET.get("id"), project=project)
+                    ProjectDebugFile.objects.filter(id=request.GET.get("id"), project_id=project.id)
                     .select_related("file")
                     .first()
                 )
@@ -332,7 +332,7 @@ class DifAssembleEndpoint(ProjectEndpoint):
             # This can under rare circumstances yield more than one file
             # which is why we use first() here instead of get().
             dif = (
-                ProjectDebugFile.objects.filter(project=project, checksum=checksum)
+                ProjectDebugFile.objects.filter(project_id=project.id, checksum=checksum)
                 .select_related("file")
                 .order_by("-id")
                 .first()

+ 62 - 0
src/sentry/migrations/0217_debugfile_remove_project_fk.py

@@ -0,0 +1,62 @@
+# Generated by Django 1.11.29 on 2021-07-02 08:31
+
+from django.db import migrations
+
+import sentry.db.models.fields.bounded
+
+
+class Migration(migrations.Migration):
+    # This flag is used to mark that a migration shouldn't be automatically run in
+    # production. We set this to True for operations that we think are risky and want
+    # someone from ops to run manually and monitor.
+    # General advice is that if in doubt, mark your migration as `is_dangerous`.
+    # Some things you should always mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that
+    #   they can be monitored. Since data migrations will now hold a transaction open
+    #   this is even more important.
+    # - Adding columns to highly active tables, even ones that are NULL.
+    is_dangerous = True
+
+    # This flag is used to decide whether to run this migration in a transaction or not.
+    # By default we prefer to run in a transaction, but for migrations where you want
+    # to `CREATE INDEX CONCURRENTLY` this needs to be set to False. Typically you'll
+    # want to create an index concurrently when adding one to an existing table.
+    # You'll also usually want to set this to `False` if you're writing a data
+    # migration, since we don't want the entire migration to run in one long-running
+    # transaction.
+    atomic = False
+
+    dependencies = [
+        ("sentry", "0216_cdc_setup_replication_index"),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            database_operations=[
+                migrations.AlterField(
+                    model_name="projectdebugfile",
+                    name="project",
+                    field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        db_constraint=False,
+                        null=True,
+                        to="sentry.Project",
+                    ),
+                )
+            ],
+            state_operations=[
+                migrations.AddField(
+                    model_name="projectdebugfile",
+                    name="project_id",
+                    field=sentry.db.models.fields.bounded.BoundedBigIntegerField(null=True),
+                ),
+                migrations.RemoveField(
+                    model_name="projectdebugfile",
+                    name="project",
+                ),
+                migrations.AlterIndexTogether(
+                    name="projectdebugfile",
+                    index_together={("project_id", "code_id"), ("project_id", "debug_id")},
+                ),
+            ],
+        )
+    ]

+ 19 - 10
src/sentry/models/debugfile.py

@@ -15,7 +15,14 @@ from symbolic.debuginfo import BcSymbolMap, UuidMapping
 
 from sentry import options
 from sentry.constants import KNOWN_DIF_FORMATS
-from sentry.db.models import BaseManager, FlexibleForeignKey, JSONField, Model, sane_repr
+from sentry.db.models import (
+    BaseManager,
+    BoundedBigIntegerField,
+    FlexibleForeignKey,
+    JSONField,
+    Model,
+    sane_repr,
+)
 from sentry.models.file import File, clear_cached_files
 from sentry.reprocessing import bump_reprocessing_revision, resolve_processing_issue
 from sentry.utils.zip import safe_extract_zip
@@ -43,9 +50,9 @@ class ProjectDebugFileManager(BaseManager):
         checksums = [x.lower() for x in checksums]
         missing = set(checksums)
 
-        found = ProjectDebugFile.objects.filter(checksum__in=checksums, project=project).values(
-            "checksum"
-        )
+        found = ProjectDebugFile.objects.filter(
+            checksum__in=checksums, project_id=project.id
+        ).values("checksum")
 
         for values in found:
             missing.discard(list(values.values())[0])
@@ -70,7 +77,7 @@ class ProjectDebugFileManager(BaseManager):
         features = frozenset(features) if features is not None else frozenset()
 
         difs = (
-            ProjectDebugFile.objects.filter(project=project, debug_id__in=debug_ids)
+            ProjectDebugFile.objects.filter(project_id=project.id, debug_id__in=debug_ids)
             .select_related("file")
             .order_by("-id")
         )
@@ -108,14 +115,14 @@ class ProjectDebugFile(Model):
     checksum = models.CharField(max_length=40, null=True, db_index=True)
     object_name = models.TextField()
     cpu_name = models.CharField(max_length=40)
-    project = FlexibleForeignKey("sentry.Project", null=True)
+    project_id = BoundedBigIntegerField(null=True)
     debug_id = models.CharField(max_length=64, db_column="uuid")
     code_id = models.CharField(max_length=64, null=True)
     data = JSONField(null=True)
     objects = ProjectDebugFileManager()
 
     class Meta:
-        index_together = (("project", "debug_id"), ("project", "code_id"))
+        index_together = (("project_id", "debug_id"), ("project_id", "code_id"))
         db_table = "sentry_projectdsymfile"
         app_label = "sentry"
 
@@ -171,7 +178,7 @@ def clean_redundant_difs(project, debug_id):
     identifier and the same or a superset of its features.
     """
     difs = (
-        ProjectDebugFile.objects.filter(project=project, debug_id=debug_id)
+        ProjectDebugFile.objects.filter(project_id=project.id, debug_id=debug_id)
         .select_related("file")
         .order_by("-id")
     )
@@ -248,7 +255,9 @@ def create_dif_from_id(project, meta, fileobj=None, file=None):
 
     dif = (
         ProjectDebugFile.objects.select_related("file")
-        .filter(project=project, debug_id=meta.debug_id, checksum=checksum, data__isnull=False)
+        .filter(
+            project_id=project.id, debug_id=meta.debug_id, checksum=checksum, data__isnull=False
+        )
         .order_by("-id")
         .first()
     )
@@ -275,7 +284,7 @@ def create_dif_from_id(project, meta, fileobj=None, file=None):
         code_id=meta.code_id,
         cpu_name=meta.arch,
         object_name=object_name,
-        project=project,
+        project_id=project.id,
         data=meta.data,
     )
 

+ 1 - 1
src/sentry/testutils/factories.py

@@ -657,7 +657,7 @@ class Factories:
         return ProjectDebugFile.objects.create(
             debug_id=debug_id,
             code_id=code_id,
-            project=project,
+            project_id=project.id,
             object_name=object_name,
             cpu_name=cpu_name or "x86_64",
             file=file,

+ 1 - 1
tests/sentry/api/endpoints/test_dif_assemble.py

@@ -102,7 +102,7 @@ class DifAssembleEndpoint(APITestCase):
             checksum=file1.checksum,
             object_name="baz.dSYM",
             cpu_name="x86_64",
-            project=self.project,
+            project_id=self.project.id,
             debug_id="df449af8-0dcd-4320-9943-ec192134d593",
             code_id="DF449AF80DCD43209943EC192134D593",
         )

+ 1 - 1
tests/sentry/deletions/test_project.py

@@ -58,7 +58,7 @@ class DeleteProjectTest(TestCase):
             code_id="codeid",
             cpu_name="cpu",
             object_name="object",
-            project=project,
+            project_id=project.id,
         )
         file_attachment = File.objects.create(name="hello.png", type="image/png")
         EventAttachment.objects.create(

+ 6 - 2
tests/sentry/tasks/test_assemble.py

@@ -70,7 +70,9 @@ class AssembleDifTest(BaseAssembleTest):
         status, _ = get_assemble_status(AssembleTask.DIF, self.project.id, total_checksum)
         assert status == ChunkFileState.OK
 
-        dif = ProjectDebugFile.objects.filter(project=self.project, checksum=total_checksum).get()
+        dif = ProjectDebugFile.objects.filter(
+            project_id=self.project.id, checksum=total_checksum
+        ).get()
 
         assert dif.file.headers == {"Content-Type": "text/x-breakpad"}
 
@@ -172,7 +174,9 @@ class AssembleDifTest(BaseAssembleTest):
         status, _ = get_assemble_status(AssembleTask.DIF, self.project.id, total_checksum)
         assert status == ChunkFileState.OK
 
-        dif = ProjectDebugFile.objects.filter(project=self.project, checksum=total_checksum).get()
+        dif = ProjectDebugFile.objects.filter(
+            project_id=self.project.id, checksum=total_checksum
+        ).get()
 
         assert dif.file.headers == {"Content-Type": "text/x-breakpad"}
         assert dif.debug_id == "67e9247c-814e-392b-a027-dbde6748fcbf-beef"

+ 2 - 2
tests/symbolicator/test_payload_full.py

@@ -123,7 +123,7 @@ class SymbolicatorResolvingIntegrationTest(RelayStoreHelper, TransactionTestCase
             file=file,
             object_name="crash.pdb",
             cpu_name="x86",
-            project=self.project,
+            project_id=self.project.id,
             debug_id="3249d99d-0c40-4931-8610-f4e4fb0b6936-1",
             code_id="5AB380779000",
         )
@@ -202,7 +202,7 @@ class SymbolicatorResolvingIntegrationTest(RelayStoreHelper, TransactionTestCase
             file=file,
             object_name="crash.pdb",
             cpu_name="x86",
-            project=self.project,
+            project_id=self.project.id,
             debug_id="3249d99d-0c40-4931-8610-f4e4fb0b6936-1",
             code_id="5AB380779000",
         )