Change unique_together to use constraints, replace use of index_together #196
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #196 +/- ##
=======================================
Coverage 93.33% 93.33%
=======================================
Files 29 29
Lines 1845 1845
=======================================
Hits 1722 1722
Misses 123 123 ☔ View full report in Codecov by Sentry. |
norkans7
force-pushed
the
update-deps
branch
2 times, most recently
from
b69715b
to
d4d3628
Compare
dash/categories/models.py
Outdated
| @@ -50,7 +50,9 @@ def __str__(self): | |||
|
|
|||
| class Meta: | |||
| ordering = ["name"] | |||
| unique_together = ("name", "org") | |||
| constraints = [ | |||
| models.UniqueConstraint(fields=["name", "org"], name="categories_category_name_318a92307e6f39fb_uniq") | |||
There was a problem hiding this comment.
I assume this weird name is to avoid a migration ? There's not many categories so it's not a problem to remove and and re-add this index.. might be nicer just to call it categories_category_name_unique
There was a problem hiding this comment.
I reused the existing index name
There was a problem hiding this comment.
I get that but I'm not sure it's worth it or that that name is guaranteed to be the one used on every deployment
dash/orgs/models.py
Outdated
| @@ -397,7 +397,9 @@ def get_time_taken(self): | |||
| return (until - self.started_on).total_seconds() | |||
|
|
|||
| class Meta: | |||
| unique_together = ("org", "task_key") | |||
| constraints = [ | |||
| models.UniqueConstraint(fields=["org", "task_key"], name="orgs_taskstate_org_id_70fc0194720c6cf5_uniq") | |||
| ), | ||
| migrations.AlterIndexTogether( | ||
| name="dashblocktype", | ||
| index_together={("slug", "name")}, |
There was a problem hiding this comment.
Is there a reason to rewrite existing migrations? I think we just let it generate new ones which I think will noop anyway.
| ), | ||
| migrations.AddIndex( | ||
| model_name="orgbackend", | ||
| index=models.Index(fields=["org", "is_active", "slug"], name="orgs_orgbac_org_id_607508_idx"), |
There was a problem hiding this comment.
I don't think there's guarantee that generated index names are the same across deploys... just let Django make a new migrations
There was a problem hiding this comment.
Or at least get Dash running on 5.0, which I think is smarter about changes to index_together than 4.2
There was a problem hiding this comment.
Yes, I am going to remove the update to 5.1 and change the index_together changes first
norkans7
force-pushed
the
update-deps
branch
2 times, most recently
from
bf9d3ac
to
ff1df2c
Compare
norkans7
changed the title
No description provided.