#34856 closed Bug (fixed)
Running tests with historical migrations that contain index together fails with TypeError.
| Reported by: | Sage Abdullah | Owned by: | Andrés Reverón Molina |
|---|---|---|---|
| Component: | Migrations | Version: | dev |
| Severity: | Release blocker | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
When running tests against Django's main branch on a project that has a historical migration that contains index_together (either via the AlterIndexTogether operation or as part of a CreateModel operation), running that migration fails with TypeError: 'class Meta' got invalid attribute(s): index_together. This is the case even if the model has already been updated to use indexes instead of index_together and new migrations have been created to update it.
To reproduce:
- Install Django < 5.1
- Start a new project and a new app
- Create a model with
index_togetherand runmakemigrations- or run
makemigrationsbefore addingindex_togetherand run it again soAlterIndexTogetheroperation is created, same result
- or run
- Write a test that uses the database (a simple test with
TestCaseand notSimpleTestCaseshould suffice) - Optional: Update the model to use
indexesinstead ofindex_togetherand runmakemigrations - Install latest Django
main - Run
python manage.py test -v3and it fails when applying the migration that containsindex_together
Reproducible example: https://github.com/laymonage/django-test-repro
This is likely because "index_together" has been removed from django.db.models.options.DEFAULT_NAMES in 2abf417c815c20f41c0868d6f66520b32347106e. Meanwhile, the historical model constructed by the migrations framework will try to build the model with the Meta.index_together option.
I'm not sure if this is expected. If it is, then that means developers have to update their old migrations to not use index_together at all, which doesn't seem to be ideal.
If it is not, the solution might be to reintroduce "index_together" in DEFAULT_NAMES even though it will be no-op. Not sure if that's all there is to it, but I'm happy to make a PR for a start.
Change History (12)
comment:1 by , 2 years ago
comment:2 by , 2 years ago
| Resolution: | → invalid |
|---|---|
| Status: | new → closed |
I was about to reply linking to the 4.2 release notes then saw you answered your own question:
To add, the 4.2 release notes do say the following though:
...
But with the problem in this ticket, it seems squashing the migrations will be mandatory?
Yes :)
follow-up: 4 comment:3 by , 2 years ago
PS: If you disagree with the removal at the end of deprecation feel free to start a thread on the Django forum: https://www.djangoproject.com/community/
comment:4 by , 2 years ago
Replying to David Sanders:
PS: If you disagree with the removal at the end of deprecation feel free to start a thread on the Django forum: https://www.djangoproject.com/community/
Thanks! I don't mind the removal itself, though I wonder if the release notes should be updated to better reflect the fact that squashing the migrations will be mandatory? The current wording seems to suggest it's optional. That said, this ticket is probably enough as additional documentation about this issue if anyone else encounters it in the future.
comment:5 by , 15 months ago
And the release notes say, "The AlterIndexTogether migration operation is now officially supported only for pre-Django 4.2 migration files. For backward compatibility reasons, it’s still part of the public API, and there’s no plan to deprecate or remove it, but it should not be used for new migrations. "
I'm not immediately seeing what purpose it serves if it crashes (see #35679). I think removing an index, e.g. AlterIndexTogether("Pony", None), still works, but I'm not sure this provides sufficient value to keep it around.
comment:6 by , 12 months ago
Hi, I'm in the process of upgrading a project from Django 4.2.x to 5.1.x and I'm facing this issue.
Every instance of Meta.index_together has already been updated to use Meta.indexes , but there are still some AlterIndexTogether in historical migrations, which happily crash with a
TypeError: 'class Meta' got invalid attribute(s): index_together error.
Unfortunately squashing migrations does not help, as AlterIndexTogether is still there after the migrations have been squashed.
Is there a recommended workaround to this? Maybe what is suggested here https://code.djangoproject.com/ticket/35679#comment:1 ?
I think this ticket and https://code.djangoproject.com/ticket/35679 were maybe closed too soon.
comment:7 by , 11 months ago
| Resolution: | invalid |
|---|---|
| Severity: | Normal → Release blocker |
| Status: | closed → new |
| Triage Stage: | Unreviewed → Accepted |
Reopening per Django forum thread.
comment:8 by , 11 months ago
| Has patch: | set |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
comment:9 by , 11 months ago
| Owner: | changed from to |
|---|
comment:10 by , 11 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
To add, the 4.2 release notes do say the following though:
But with the problem in this ticket, it seems squashing the migrations will be mandatory?