From 604c599f52fc220068ecdfa81d4c6e4b4a3bea95 Mon Sep 17 00:00:00 2001 From: Daniel Yu Date: Thu, 22 Feb 2024 10:55:41 -0500 Subject: [PATCH] squashed: address pr comments, removed redundant constraint and change field type add sql migration comment allow empty array for override ids use strings for override keys instead fix-typo --- .../rollouts/migrations/0001_initial.py | 66 ++++++------------- shared/django_apps/rollouts/models.py | 24 +++---- 2 files changed, 28 insertions(+), 62 deletions(-) diff --git a/shared/django_apps/rollouts/migrations/0001_initial.py b/shared/django_apps/rollouts/migrations/0001_initial.py index 6e095c526..c55f1def1 100644 --- a/shared/django_apps/rollouts/migrations/0001_initial.py +++ b/shared/django_apps/rollouts/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.7 on 2024-02-21 20:13 +# Generated by Django 4.2.7 on 2024-02-23 16:43 import django.contrib.postgres.fields import django.db.models.deletion @@ -17,30 +17,11 @@ class Migration(migrations.Migration): # -- # -- Create model FeatureFlag # -- - # CREATE TABLE "feature_flags" ("name" varchar(200) NOT NULL PRIMARY KEY, "proportion" decimal NOT NULL, "salt" varchar(10000) NOT NULL); + # CREATE TABLE "feature_flags" ("name" varchar(200) NOT NULL PRIMARY KEY, "proportion" decimal NOT NULL, "salt" varchar(32) NOT NULL); # -- # -- Create model FeatureFlagVariant # -- - # CREATE TABLE "feature_flag_variants" ("name" varchar(200) NOT NULL PRIMARY KEY, "proportion" decimal NOT NULL, "enabled" bool NOT NULL, "override_owner_ids" integer[] NOT NULL, "override_repo_ids" integer[] NOT NULL, "feature_flag_id" varchar(200) NOT NULL REFERENCES "feature_flags" ("name") DEFERRABLE INITIALLY DEFERRED); - # -- - # -- Create constraint feature_flag_name on model featureflag - # -- - # CREATE TABLE "new__feature_flags" ("name" varchar(200) NOT NULL PRIMARY KEY, "proportion" decimal NOT NULL, "salt" varchar(10000) NOT NULL, CONSTRAINT "feature_flag_name" UNIQUE ("name")); - # INSERT INTO "new__feature_flags" ("name", "proportion", "salt") SELECT "name", "proportion", "salt" FROM "feature_flags"; - # DROP TABLE "feature_flags"; - # ALTER TABLE "new__feature_flags" RENAME TO "feature_flags"; - # CREATE INDEX "feature_flag_variants_feature_flag_id_fa3a4c02" ON "feature_flag_variants" ("feature_flag_id"); - # -- - # -- Create index feature_fla_feature_15a078_idx on field(s) feature_flag of model featureflagvariant - # -- - # CREATE INDEX "feature_fla_feature_15a078_idx" ON "feature_flag_variants" ("feature_flag_id"); - # -- - # -- Create constraint feature_flag_variant_name on model featureflagvariant - # -- - # CREATE TABLE "new__feature_flag_variants" ("name" varchar(200) NOT NULL PRIMARY KEY, "proportion" decimal NOT NULL, "enabled" bool NOT NULL, "override_owner_ids" integer[] NOT NULL, "override_repo_ids" integer[] NOT NULL, "feature_flag_id" varchar(200) NOT NULL REFERENCES "feature_flags" ("name") DEFERRABLE INITIALLY DEFERRED, CONSTRAINT "feature_flag_variant_name" UNIQUE ("name")); - # INSERT INTO "new__feature_flag_variants" ("name", "proportion", "enabled", "override_owner_ids", "override_repo_ids", "feature_flag_id") SELECT "name", "proportion", "enabled", "override_owner_ids", "override_repo_ids", "feature_flag_id" FROM "feature_flag_variants"; - # DROP TABLE "feature_flag_variants"; - # ALTER TABLE "new__feature_flag_variants" RENAME TO "feature_flag_variants"; + # CREATE TABLE "feature_flag_variants" ("name" varchar(200) NOT NULL PRIMARY KEY, "proportion" decimal NOT NULL, "value" text NOT NULL CHECK ((JSON_VALID("value") OR "value" IS NULL)), "override_owner_keys" varchar(200)[] NOT NULL, "override_repo_keys" varchar(200)[] NOT NULL, "feature_flag_id" varchar(200) NOT NULL REFERENCES "feature_flags" ("name") DEFERRABLE INITIALLY DEFERRED); # CREATE INDEX "feature_flag_variants_feature_flag_id_fa3a4c02" ON "feature_flag_variants" ("feature_flag_id"); # CREATE INDEX "feature_fla_feature_15a078_idx" ON "feature_flag_variants" ("feature_flag_id"); # COMMIT; @@ -61,7 +42,7 @@ class Migration(migrations.Migration): "salt", models.CharField( default=shared.django_apps.rollouts.models.default_random_salt, - max_length=10000, + max_length=32, ), ), ], @@ -80,17 +61,23 @@ class Migration(migrations.Migration): "proportion", models.DecimalField(decimal_places=3, default=0, max_digits=4), ), - ("enabled", models.BooleanField(default=False)), + ("value", models.JSONField(default=False)), ( - "override_owner_ids", + "override_owner_keys", django.contrib.postgres.fields.ArrayField( - base_field=models.IntegerField(), default=list, size=None + base_field=models.CharField(max_length=200), + blank=True, + default=list, + size=None, ), ), ( - "override_repo_ids", + "override_repo_keys", django.contrib.postgres.fields.ArrayField( - base_field=models.IntegerField(), default=list, size=None + base_field=models.CharField(max_length=200), + blank=True, + default=list, + size=None, ), ), ( @@ -104,24 +91,11 @@ class Migration(migrations.Migration): ], options={ "db_table": "feature_flag_variants", + "indexes": [ + models.Index( + fields=["feature_flag"], name="feature_fla_feature_15a078_idx" + ) + ], }, ), - migrations.AddConstraint( - model_name="featureflag", - constraint=models.UniqueConstraint( - fields=("name",), name="feature_flag_name" - ), - ), - migrations.AddIndex( - model_name="featureflagvariant", - index=models.Index( - fields=["feature_flag"], name="feature_fla_feature_15a078_idx" - ), - ), - migrations.AddConstraint( - model_name="featureflagvariant", - constraint=models.UniqueConstraint( - fields=("name",), name="feature_flag_variant_name" - ), - ), ] diff --git a/shared/django_apps/rollouts/models.py b/shared/django_apps/rollouts/models.py index 4c850d52c..7d1227be4 100644 --- a/shared/django_apps/rollouts/models.py +++ b/shared/django_apps/rollouts/models.py @@ -21,14 +21,10 @@ class FeatureFlag(models.Model): name = models.CharField(max_length=200, primary_key=True) proportion = models.DecimalField(default=0, decimal_places=3, max_digits=4) - salt = models.CharField(max_length=10000, default=default_random_salt) + salt = models.CharField(max_length=32, default=default_random_salt) class Meta: db_table = "feature_flags" - constraints = [ - models.UniqueConstraint(fields=["name"], name="feature_flag_name") - ] - # TODO: assert that variant proportions sum to 1 class FeatureFlagVariant(models.Model): @@ -40,25 +36,21 @@ class FeatureFlagVariant(models.Model): the proportions of the corresponding `FeatureFlagVariant`s sum to 1. """ - name = models.CharField(max_length=200, primary_key=True) + name = models.CharField(max_length=200, primary_key=True) feature_flag = models.ForeignKey( "FeatureFlag", on_delete=models.CASCADE, related_name="variants" ) proportion = models.DecimalField(default=0, decimal_places=3, max_digits=4) - enabled = models.BooleanField(default=False) + value = models.JSONField(default=False) - # Weak foreign keys to Owner and Respository models respectively - override_owner_ids = fields.ArrayField( - base_field=models.IntegerField(), default=list + # Weak foreign keys to Owner and Respository models respectively. Could be either a slug or ID + override_owner_keys = fields.ArrayField( + base_field=models.CharField(max_length=200), default=list, blank=True ) - override_repo_ids = fields.ArrayField( - base_field=models.IntegerField(), default=list + override_repo_keys = fields.ArrayField( + base_field=models.CharField(max_length=200), default=list, blank=True ) - # TODO: maybe add more fields for more granularity on feature variants. EG: featureA uses value 10 vs featureB uses value 100 class Meta: db_table = "feature_flag_variants" - constraints = [ - models.UniqueConstraint(fields=["name"], name="feature_flag_variant_name") - ] indexes = [models.Index(fields=["feature_flag"])]