Browse Source

ref: express the indexes in release models in django terms (#64355)

this resolves a django 4.x warning about `index_together` which becomes
fatal in django 5.x

<!-- Describe your PR here. -->
anthony sottile 1 year ago
parent
commit
7e3498090f

+ 2 - 1
.github/workflows/backend.yml

@@ -139,7 +139,8 @@ jobs:
 
       - name: run tests
         run: |
-          PYTEST_ADDOPTS="$PYTEST_ADDOPTS -m migrations --migrations" make test-python-ci
+          # historic migrations trigger some warnings
+          PYTEST_ADDOPTS="$PYTEST_ADDOPTS -m migrations --migrations -W ignore" make test-python-ci
 
       # Upload coverage data even if running the tests step fails since
       # it reduces large coverage fluctuations

+ 1 - 1
migrations_lockfile.txt

@@ -9,5 +9,5 @@ feedback: 0004_index_together
 hybridcloud: 0009_make_user_id_optional_for_slug_reservation_replica
 nodestore: 0002_nodestore_no_dictfield
 replays: 0004_index_together
-sentry: 0641_backfill_group_attributes
+sentry: 0642_index_together_release
 social_auth: 0002_default_auto_field

+ 0 - 3
pyproject.toml

@@ -32,9 +32,6 @@ filterwarnings = [
     # Consider all warnings to be errors other than the ignored ones.
     "error",
 
-    # TODO: after upgrading to django 4.x
-    "ignore:'index_together' is deprecated. Use 'Meta.indexes' in.*",
-
     # a bunch of google packages use legacy namespace packages
     "ignore:pkg_resources is deprecated as an API",
     "ignore:Deprecated call to `pkg_resources.declare_namespace\\('google.*'\\)`.",

+ 5 - 0
src/sentry/db/models/indexes.py

@@ -0,0 +1,5 @@
+from django.db.models.indexes import Index
+
+
+class IndexWithPostgresNameLimits(Index):
+    max_name_length = 63

+ 105 - 0
src/sentry/migrations/0642_index_together_release.py

@@ -0,0 +1,105 @@
+# Generated by Django 4.2.8 on 2024-02-01 11:45
+
+from django.db import migrations, models
+
+import sentry.db.models.indexes
+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", "0641_backfill_group_attributes"),
+    ]
+
+    operations = [
+        migrations.RenameIndex(
+            model_name="release",
+            new_name="sentry_rele_organiz_6975e7_idx",
+            old_fields=("organization", "status"),
+        ),
+        migrations.RenameIndex(
+            model_name="release",
+            new_name="sentry_rele_organiz_6b035f_idx",
+            old_fields=("organization", "build_number"),
+        ),
+        migrations.RenameIndex(
+            model_name="release",
+            new_name="sentry_rele_organiz_4ed947_idx",
+            old_fields=("organization", "date_added"),
+        ),
+        migrations.RenameIndex(
+            model_name="release",
+            new_name="sentry_rele_organiz_ffeeb2_idx",
+            old_fields=("organization", "build_code"),
+        ),
+        migrations.RenameIndex(
+            model_name="releaseproject",
+            new_name="sentry_rele_project_2ca122_idx",
+            old_fields=("project", "unadopted"),
+        ),
+        migrations.RenameIndex(
+            model_name="releaseproject",
+            new_name="sentry_rele_project_3143eb_idx",
+            old_fields=("project", "first_seen_transaction"),
+        ),
+        migrations.RenameIndex(
+            model_name="releaseproject",
+            new_name="sentry_rele_project_a80825_idx",
+            old_fields=("project", "adopted"),
+        ),
+        migrations.SeparateDatabaseAndState(
+            # fancy noop, we're now defining this index in django words
+            database_operations=[],
+            state_operations=[
+                migrations.AlterIndexTogether(
+                    name="release",
+                    index_together=set(),
+                ),
+                migrations.AddIndex(
+                    model_name="release",
+                    index=sentry.db.models.indexes.IndexWithPostgresNameLimits(
+                        models.F("organization"),
+                        models.F("package"),
+                        models.OrderBy(models.F("major"), descending=True),
+                        models.OrderBy(models.F("minor"), descending=True),
+                        models.OrderBy(models.F("patch"), descending=True),
+                        models.OrderBy(models.F("revision"), descending=True),
+                        models.OrderBy(
+                            models.Case(models.When(prerelease="", then=1), default=0),
+                            descending=True,
+                        ),
+                        models.OrderBy(models.F("prerelease"), descending=True),
+                        name="sentry_release_semver_by_package_idx",
+                    ),
+                ),
+                migrations.AddIndex(
+                    model_name="release",
+                    index=models.Index(
+                        models.F("organization"),
+                        models.OrderBy(models.F("major"), descending=True),
+                        models.OrderBy(models.F("minor"), descending=True),
+                        models.OrderBy(models.F("patch"), descending=True),
+                        models.OrderBy(models.F("revision"), descending=True),
+                        models.OrderBy(
+                            models.Case(models.When(prerelease="", then=1), default=0),
+                            descending=True,
+                        ),
+                        models.OrderBy(models.F("prerelease"), descending=True),
+                        name="sentry_release_semver_idx",
+                    ),
+                ),
+            ],
+        ),
+    ]

+ 32 - 18
src/sentry/models/release.py

@@ -34,6 +34,7 @@ from sentry.db.models import (
     sane_repr,
 )
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
+from sentry.db.models.indexes import IndexWithPostgresNameLimits
 from sentry.db.models.manager import BaseManager
 from sentry.db.postgres.transactions import in_test_hide_transaction_boundary
 from sentry.exceptions import InvalidSearchQuery
@@ -115,10 +116,10 @@ class ReleaseProject(Model):
     class Meta:
         app_label = "sentry"
         db_table = "sentry_release_project"
-        index_together = (
-            ("project", "adopted"),
-            ("project", "unadopted"),
-            ("project", "first_seen_transaction"),
+        indexes = (
+            models.Index(fields=("project", "adopted")),
+            models.Index(fields=("project", "unadopted")),
+            models.Index(fields=("project", "first_seen_transaction")),
         )
         unique_together = (("project", "release"),)
 
@@ -530,25 +531,38 @@ class Release(Model):
         app_label = "sentry"
         db_table = "sentry_release"
         unique_together = (("organization", "version"),)
-        # TODO(django2.2): Note that we create this index with each column ordered
-        # descending. Django 2.2 allows us to specify functional indexes, which should
-        # allow us to specify this on the model.
-        # We also use a functional index to order `prerelease` according to semver rules,
-        # which we can't express here for now.
-        index_together = (
-            ("organization", "package", "major", "minor", "patch", "revision", "prerelease"),
-            ("organization", "major", "minor", "patch", "revision", "prerelease"),
-            ("organization", "build_code"),
-            ("organization", "build_number"),
-            ("organization", "date_added"),
-            ("organization", "status"),
-        )
         indexes = [
             models.Index(
                 fields=["organization", "version"],
                 opclasses=["", "text_pattern_ops"],
                 name="sentry_release_version_btree",
-            )
+            ),
+            # We also use a functional index to order `prerelease` according to semver rules,
+            IndexWithPostgresNameLimits(
+                "organization",
+                "package",
+                F("major").desc(),
+                F("minor").desc(),
+                F("patch").desc(),
+                F("revision").desc(),
+                Case(When(prerelease="", then=1), default=0).desc(),
+                F("prerelease").desc(),
+                name="sentry_release_semver_by_package_idx",
+            ),
+            models.Index(
+                "organization",
+                F("major").desc(),
+                F("minor").desc(),
+                F("patch").desc(),
+                F("revision").desc(),
+                Case(When(prerelease="", then=1), default=0).desc(),
+                F("prerelease").desc(),
+                name="sentry_release_semver_idx",
+            ),
+            models.Index(fields=("organization", "build_code")),
+            models.Index(fields=("organization", "build_number")),
+            models.Index(fields=("organization", "date_added")),
+            models.Index(fields=("organization", "status")),
         ]
 
     __repr__ = sane_repr("organization_id", "version")