From 38f7bc239a87214a06869a5d8aec5d3ad6c80d4a Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Sun, 9 Apr 2023 23:00:18 -0700 Subject: [PATCH 1/6] ref(hc): Adding back in actor constraint idempotently Also adds in org and integration non null --- migrations_lockfile.txt | 2 +- .../migrations/0416_add_actor_constraints.py | 76 +++++++++++++++++++ .../models/integrations/pagerduty_service.py | 2 +- .../repository_project_path_config.py | 4 +- 4 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 src/sentry/migrations/0416_add_actor_constraints.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index abed7421ecc057..e020945c0ba944 100644 --- a/migrations_lockfile.txt +++ b/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: 0415_backfill_actor_team_and_user +sentry: 0416_add_actor_constraint social_auth: 0001_initial diff --git a/src/sentry/migrations/0416_add_actor_constraints.py b/src/sentry/migrations/0416_add_actor_constraints.py new file mode 100644 index 00000000000000..e2d26e4fc1fcf7 --- /dev/null +++ b/src/sentry/migrations/0416_add_actor_constraints.py @@ -0,0 +1,76 @@ +# Generated by Django 2.2.28 on 2023-01-31 20:37 + +from django.db import migrations + +import sentry.db.models.fields.bounded +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 = True + + dependencies = [ + ("sentry", "0415_backfill_actor_team_and_user"), + ] + + operations = ( + [ + migrations.RunSQL( + sql=line, + reverse_sql="", + hints={"tables": ["sentry_actor"]}, + ) + for line in """ +ALTER TABLE "sentry_actor" DROP CONSTRAINT IF EXISTS "sentry_actor_team_id_6ca8eba5_fk_sentry_team_id"; +ALTER TABLE "sentry_actor" DROP CONSTRAINT IF EXISTS "sentry_actor_team_id_6ca8eba5_uniq"; +DROP INDEX CONCURRENTLY IF EXISTS "sentry_actor_team_id_6ca8eba5"; +DROP INDEX CONCURRENTLY IF EXISTS "sentry_actor_team_id_6ca8eba5_uniq"; + +CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS "sentry_actor_team_id_6ca8eba5_uniq" ON "sentry_actor" ("team_id"); +ALTER TABLE "sentry_actor" ADD CONSTRAINT "sentry_actor_team_id_6ca8eba5_fk_sentry_team_id" FOREIGN KEY ("team_id") REFERENCES "sentry_team" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID; +ALTER TABLE "sentry_actor" VALIDATE CONSTRAINT "sentry_actor_team_id_6ca8eba5_fk_sentry_team_id"; + +ALTER TABLE "sentry_actor" DROP CONSTRAINT IF EXISTS "sentry_actor_user_id_c832ff63_uniq"; +DROP INDEX CONCURRENTLY IF EXISTS "sentry_actor_user_id_c832ff63"; +DROP INDEX CONCURRENTLY IF EXISTS "sentry_actor_user_id_c832ff63_uniq"; +CREATE UNIQUE INDEX CONCURRENTLY "sentry_actor_user_id_c832ff63_uniq" ON "sentry_actor" ("user_id"); + """.splitlines() + if line.strip() + ] + + [ + migrations.AlterField( + model_name="pagerdutyservice", + name="organization_id", + field=sentry.db.models.fields.bounded.BoundedBigIntegerField( + db_index=True, null=False + ), + ), + migrations.AlterField( + model_name="repositoryprojectpathconfig", + name="organization_id", + field=sentry.db.models.fields.bounded.BoundedBigIntegerField( + db_index=True, null=False + ), + ), + migrations.AlterField( + model_name="pagerdutyservice", + name="integration_id", + field=sentry.db.models.fields.bounded.BoundedBigIntegerField(null=False), + ), + migrations.AlterField( + model_name="repositoryprojectpathconfig", + name="integration_id", + field=sentry.db.models.fields.bounded.BoundedBigIntegerField(null=False), + ), + ] + ) diff --git a/src/sentry/models/integrations/pagerduty_service.py b/src/sentry/models/integrations/pagerduty_service.py index 3b98346142286d..38fee535967bd1 100644 --- a/src/sentry/models/integrations/pagerduty_service.py +++ b/src/sentry/models/integrations/pagerduty_service.py @@ -19,7 +19,7 @@ class PagerDutyService(DefaultFieldsModel, OrganizationIntegrityBackfillMixin): organization_integration = FlexibleForeignKey("sentry.OrganizationIntegration") organization_id = BoundedBigIntegerField(null=True, db_index=True) # From a region point of view, you really only have per organization scoping. - integration_id = BoundedBigIntegerField(null=True, db_index=False) + integration_id = BoundedBigIntegerField(null=False, db_index=False) integration_key = models.CharField(max_length=255) service_name = models.CharField(max_length=255) date_added = models.DateTimeField(default=timezone.now) diff --git a/src/sentry/models/integrations/repository_project_path_config.py b/src/sentry/models/integrations/repository_project_path_config.py index ff80964698ec49..b7310cb9e55e82 100644 --- a/src/sentry/models/integrations/repository_project_path_config.py +++ b/src/sentry/models/integrations/repository_project_path_config.py @@ -20,9 +20,9 @@ class RepositoryProjectPathConfig(DefaultFieldsModel, OrganizationIntegrityBackf project = FlexibleForeignKey("sentry.Project", db_constraint=False) organization_integration = FlexibleForeignKey("sentry.OrganizationIntegration") - organization_id = BoundedBigIntegerField(null=True, db_index=True) + organization_id = BoundedBigIntegerField(null=False, db_index=True) # From a region point of view, you really only have per organization scoping. - integration_id = BoundedBigIntegerField(null=True, db_index=False) + integration_id = BoundedBigIntegerField(null=False, db_index=False) stack_root = models.TextField() source_root = models.TextField() default_branch = models.TextField(null=True) From d41ff5ece1eb115d83435cf2cd617b64fafa846c Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Sun, 9 Apr 2023 23:05:38 -0700 Subject: [PATCH 2/6] Fix model --- src/sentry/models/integrations/pagerduty_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/models/integrations/pagerduty_service.py b/src/sentry/models/integrations/pagerduty_service.py index 38fee535967bd1..6ba5f4944f080b 100644 --- a/src/sentry/models/integrations/pagerduty_service.py +++ b/src/sentry/models/integrations/pagerduty_service.py @@ -17,7 +17,7 @@ class PagerDutyService(DefaultFieldsModel, OrganizationIntegrityBackfillMixin): __include_in_export__ = False organization_integration = FlexibleForeignKey("sentry.OrganizationIntegration") - organization_id = BoundedBigIntegerField(null=True, db_index=True) + organization_id = BoundedBigIntegerField(null=False, db_index=True) # From a region point of view, you really only have per organization scoping. integration_id = BoundedBigIntegerField(null=False, db_index=False) integration_key = models.CharField(max_length=255) From 594e2b8c11252581af367f8b8c7fa4fa8352c13a Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Sun, 9 Apr 2023 23:21:54 -0700 Subject: [PATCH 3/6] Fix lock --- migrations_lockfile.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index e020945c0ba944..a8559202dfb9a2 100644 --- a/migrations_lockfile.txt +++ b/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: 0416_add_actor_constraint +sentry: 0416_add_actor_constraints social_auth: 0001_initial From 68e356d0509905a6dd0820440faeaf87f0781a5f Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Mon, 10 Apr 2023 03:04:07 -0700 Subject: [PATCH 4/6] Fix backfill mixin --- src/sentry/models/integrations/pagerduty_service.py | 2 +- .../models/integrations/repository_project_path_config.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/models/integrations/pagerduty_service.py b/src/sentry/models/integrations/pagerduty_service.py index 6ba5f4944f080b..e29b2942154047 100644 --- a/src/sentry/models/integrations/pagerduty_service.py +++ b/src/sentry/models/integrations/pagerduty_service.py @@ -13,7 +13,7 @@ @region_silo_only_model -class PagerDutyService(DefaultFieldsModel, OrganizationIntegrityBackfillMixin): +class PagerDutyService(OrganizationIntegrityBackfillMixin, DefaultFieldsModel): __include_in_export__ = False organization_integration = FlexibleForeignKey("sentry.OrganizationIntegration") diff --git a/src/sentry/models/integrations/repository_project_path_config.py b/src/sentry/models/integrations/repository_project_path_config.py index b7310cb9e55e82..c2ec8674743dcc 100644 --- a/src/sentry/models/integrations/repository_project_path_config.py +++ b/src/sentry/models/integrations/repository_project_path_config.py @@ -13,7 +13,7 @@ @region_silo_only_model -class RepositoryProjectPathConfig(DefaultFieldsModel, OrganizationIntegrityBackfillMixin): +class RepositoryProjectPathConfig(OrganizationIntegrityBackfillMixin, DefaultFieldsModel): __include_in_export__ = False repository = FlexibleForeignKey("sentry.Repository") From 113220377d5bc6c0061317b4dd7f5cd134163adc Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Mon, 10 Apr 2023 15:49:11 -0700 Subject: [PATCH 5/6] Fixing tests --- .../migrations/0416_add_actor_constraints.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/sentry/migrations/0416_add_actor_constraints.py b/src/sentry/migrations/0416_add_actor_constraints.py index e2d26e4fc1fcf7..14418cbf939969 100644 --- a/src/sentry/migrations/0416_add_actor_constraints.py +++ b/src/sentry/migrations/0416_add_actor_constraints.py @@ -43,11 +43,25 @@ class Migration(CheckedMigration): ALTER TABLE "sentry_actor" DROP CONSTRAINT IF EXISTS "sentry_actor_user_id_c832ff63_uniq"; DROP INDEX CONCURRENTLY IF EXISTS "sentry_actor_user_id_c832ff63"; DROP INDEX CONCURRENTLY IF EXISTS "sentry_actor_user_id_c832ff63_uniq"; -CREATE UNIQUE INDEX CONCURRENTLY "sentry_actor_user_id_c832ff63_uniq" ON "sentry_actor" ("user_id"); +CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS "sentry_actor_user_id_c832ff63_uniq" ON "sentry_actor" ("user_id"); """.splitlines() if line.strip() ] + [ + migrations.RunSQL( + sql="SELECT 1", + reverse_sql=""" +ALTER TABLE "sentry_actor" ADD CONSTRAINT "sentry_actor_team_id_6ca8eba5_uniq" UNIQUE USING INDEX "sentry_actor_team_id_6ca8eba5_uniq"; + """, + hints={"tables": ["sentry_actor"]}, + ), + migrations.RunSQL( + sql="SELECT 1", + reverse_sql=""" +ALTER TABLE "sentry_actor" ADD CONSTRAINT "sentry_actor_user_id_c832ff63_uniq" UNIQUE USING INDEX "sentry_actor_user_id_c832ff63_uniq"; + """, + hints={"tables": ["sentry_actor"]}, + ), migrations.AlterField( model_name="pagerdutyservice", name="organization_id", From 488d14eb1f7f000fe973dfd979515486658636a3 Mon Sep 17 00:00:00 2001 From: Zachary Collins Date: Mon, 10 Apr 2023 21:54:41 -0700 Subject: [PATCH 6/6] Thing --- .../migrations/0418_add_actor_constraints.py | 25 ------------------- .../models/integrations/pagerduty_service.py | 4 +-- .../repository_project_path_config.py | 4 +-- 3 files changed, 4 insertions(+), 29 deletions(-) diff --git a/src/sentry/migrations/0418_add_actor_constraints.py b/src/sentry/migrations/0418_add_actor_constraints.py index 3f9da70d9b5945..82ccdfabfe794f 100644 --- a/src/sentry/migrations/0418_add_actor_constraints.py +++ b/src/sentry/migrations/0418_add_actor_constraints.py @@ -2,7 +2,6 @@ from django.db import migrations -import sentry.db.models.fields.bounded from sentry.new_migrations.migrations import CheckedMigration @@ -62,29 +61,5 @@ class Migration(CheckedMigration): """, hints={"tables": ["sentry_actor"]}, ), - migrations.AlterField( - model_name="pagerdutyservice", - name="organization_id", - field=sentry.db.models.fields.bounded.BoundedBigIntegerField( - db_index=True, null=False - ), - ), - migrations.AlterField( - model_name="repositoryprojectpathconfig", - name="organization_id", - field=sentry.db.models.fields.bounded.BoundedBigIntegerField( - db_index=True, null=False - ), - ), - migrations.AlterField( - model_name="pagerdutyservice", - name="integration_id", - field=sentry.db.models.fields.bounded.BoundedBigIntegerField(null=False), - ), - migrations.AlterField( - model_name="repositoryprojectpathconfig", - name="integration_id", - field=sentry.db.models.fields.bounded.BoundedBigIntegerField(null=False), - ), ] ) diff --git a/src/sentry/models/integrations/pagerduty_service.py b/src/sentry/models/integrations/pagerduty_service.py index e29b2942154047..0cc7d747f8a5d7 100644 --- a/src/sentry/models/integrations/pagerduty_service.py +++ b/src/sentry/models/integrations/pagerduty_service.py @@ -17,9 +17,9 @@ class PagerDutyService(OrganizationIntegrityBackfillMixin, DefaultFieldsModel): __include_in_export__ = False organization_integration = FlexibleForeignKey("sentry.OrganizationIntegration") - organization_id = BoundedBigIntegerField(null=False, db_index=True) + organization_id = BoundedBigIntegerField(null=True, db_index=True) # From a region point of view, you really only have per organization scoping. - integration_id = BoundedBigIntegerField(null=False, db_index=False) + integration_id = BoundedBigIntegerField(null=True, db_index=False) integration_key = models.CharField(max_length=255) service_name = models.CharField(max_length=255) date_added = models.DateTimeField(default=timezone.now) diff --git a/src/sentry/models/integrations/repository_project_path_config.py b/src/sentry/models/integrations/repository_project_path_config.py index c2ec8674743dcc..be451fbb862607 100644 --- a/src/sentry/models/integrations/repository_project_path_config.py +++ b/src/sentry/models/integrations/repository_project_path_config.py @@ -20,9 +20,9 @@ class RepositoryProjectPathConfig(OrganizationIntegrityBackfillMixin, DefaultFie project = FlexibleForeignKey("sentry.Project", db_constraint=False) organization_integration = FlexibleForeignKey("sentry.OrganizationIntegration") - organization_id = BoundedBigIntegerField(null=False, db_index=True) + organization_id = BoundedBigIntegerField(null=True, db_index=True) # From a region point of view, you really only have per organization scoping. - integration_id = BoundedBigIntegerField(null=False, db_index=False) + integration_id = BoundedBigIntegerField(null=True, db_index=False) stack_root = models.TextField() source_root = models.TextField() default_branch = models.TextField(null=True)