-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update django 5.1 #197
Update django 5.1 #197
Conversation
migrations.AlterIndexTogether( | ||
name="dashblock", | ||
index_together={("org", "is_active", "dashblock_type", "priority")}, | ||
migrations.AddIndex( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to adjust these old migrations to not use index_together
as that throw the error
TypeError: 'class Meta' got invalid attribute(s): index_together
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's true.. Django 5.1 still has AlterIndexTogether
for backwards compatibility - you just can't use index_together
on a model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I will step through the codes and see why that error is being thrown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to squash migrations before moving to 5.1 so that no index_together is used in historical migrations
https://docs.djangoproject.com/en/5.1/releases/4.2/#index-together-option-is-deprecated-in-favor-of-indexes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rowanseymour even with squashing the migrations we still need to adjust them to not use index_together
No other solution has been provided even on the tickets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in our case we can just even skip the squash migrations and just adjust the past migrations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And editing the squashed migrations or existing migrations is what is suggested by https://code.djangoproject.com/ticket/34525#comment:17
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh... ok then sorry I was wrong about that.. I think go ahead and manually tweak old migrations if that's the easiest way forward
9936f8b
to
4ba35ef
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #197 +/- ##
=======================================
Coverage 93.33% 93.33%
=======================================
Files 29 29
Lines 1845 1845
=======================================
Hits 1722 1722
Misses 123 123 ☔ View full report in Codecov by Sentry. |
4ba35ef
to
1860a8f
Compare
1860a8f
to
4be9e6e
Compare
4be9e6e
to
ad81190
Compare
ad81190
to
c1b5ee0
Compare
c1b5ee0
to
d1c17e3
Compare
once #196 is merged