Opened 8 months ago
Closed 6 months ago
#35424 closed Bug (fixed)
Migration autodetector fails when order_with_respect_to is removed, but an _order field remains
Reported by: | Stuart Attenborrow | Owned by: | Daniel Patrick |
---|---|---|---|
Component: | Migrations | Version: | 4.2 |
Severity: | Normal | 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
We have a project with a large number of migrations that began failing during migrate/makemigrations due to order_with_respect_to having been removed, and an explicit _order field added. The historic migration contains this operation:
migrations.AlterOrderWithRespectTo( name='choice', order_with_respect_to=None, ),
This causes an issue with the code introduced in commit https://github.com/django/django/commit/aa4acc164d1247c0de515c959f7b09648b57dc42 which was subsequently "fixed" in https://code.djangoproject.com/ticket/33449 but remains failing in the case described above.
A minimal reproducible case has been created here: https://github.com/stuarta0/django-order-with-respect-to based on the django docs polls app.
A possible solution may be to fall back to field_name if order_with_respect_to is either missing, or present with None (which is what occurs above):
def get_field(self, field_name): if field_name == "_order": field_name = self.options.get("order_with_respect_to") or field_name return self.fields[field_name]
Attachments (1)
Change History (12)
comment:1 by , 8 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 8 months ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
Triage Stage: | Accepted → Unreviewed |
comment:3 by , 8 months ago
Thanks for looking into it. Due to the error you're seeing, the migrations had to be staggered to achieve the changes. e.g.:
- 0002_alter_choice_order_with_respect_to: arranging the test.
- 0003_choice_order: we wanted to move to our own ordering, but retain the data from the existing hidden field. Removing order_with_respect_to would've lost the data, so we added a placeholder field to store the result, and copied the values across with a simple python migration.
- 0004_alter_choice_options_and_more: the order_with_respect_to field can be removed, which drops the hidden column.
- 0005_alter_choice_options_rename_order_choice__order: finally, we can rename our placeholder field.
Since the code is 8 years old and written by another author, I can't give you an explicit reason why the "_order" name was reused other than my assumption that "order" is a reserved word and "_order" was good enough for django internals at the time. However, it hasn't been an issue because django never had code that was structured in the way it is now.
The answer for new projects is probably to remake or squash migrations to avoid it, but it's quite difficult on a legacy project with this scale of migrations. My suggested change in the original report fixes the issue for this case, but I haven't run django tests to see if it fails in other cases.
comment:4 by , 8 months ago
Resolution: | needsinfo → worksforme |
---|
Hi Stuart,
I have cloned your test repo installed Django 4.2, ran migrations and they all apply without an error for me 🤔
I think you also need to share the error that you are seeing, I can't determine that Django is at fault here.
by , 8 months ago
Attachment: | django-order-with-respect-to.zip added |
---|
comment:5 by , 8 months ago
Error shown below. I've added a zip that contains the same project, but with docker compose, and a poetry lock file to ensure we're looking at the same thing. Note I can reproduce this both in a docker container, and on a production EC2 instance using system python and virtual envs.
Edit: to run migrate I'm shelling into the container with docker compose run mysite bash
followed by the command below:
/code/mysite# python manage.py migrate Operations to perform: Apply all migrations: admin, auth, contenttypes, polls, sessions Running migrations: No migrations to apply. Traceback (most recent call last): File "/code/mysite/manage.py", line 22, in <module> main() File "/code/mysite/manage.py", line 18, in main execute_from_command_line(sys.argv) File "/opt/venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line utility.execute() File "/opt/venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 436, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "/opt/venv/lib/python3.11/site-packages/django/core/management/base.py", line 412, in run_from_argv self.execute(*args, **cmd_options) File "/opt/venv/lib/python3.11/site-packages/django/core/management/base.py", line 458, in execute output = self.handle(*args, **options) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/opt/venv/lib/python3.11/site-packages/django/core/management/base.py", line 106, in wrapper res = handle_func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/opt/venv/lib/python3.11/site-packages/django/core/management/commands/migrate.py", line 335, in handle changes = autodetector.changes(graph=executor.loader.graph) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/opt/venv/lib/python3.11/site-packages/django/db/migrations/autodetector.py", line 46, in changes changes = self._detect_changes(convert_apps, graph) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/opt/venv/lib/python3.11/site-packages/django/db/migrations/autodetector.py", line 197, in _detect_changes self.generate_altered_fields() File "/opt/venv/lib/python3.11/site-packages/django/db/migrations/autodetector.py", line 1117, in generate_altered_fields old_field = self.from_state.models[app_label, old_model_name].get_field( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/opt/venv/lib/python3.11/site-packages/django/db/migrations/state.py", line 765, in get_field return self.fields[field_name] ~~~~~~~~~~~^^^^^^^^^^^^ KeyError: None
comment:6 by , 8 months ago
Easy pickings: | unset |
---|---|
Triage Stage: | Unreviewed → Accepted |
Hi thank you Stuart for the extra context, I'm able to replicate your error too.
Note for others, it can "migrate" without error on the first run but then error on future calls to migrate.
comment:7 by , 8 months ago
Resolution: | worksforme |
---|---|
Status: | closed → new |
comment:8 by , 6 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:10 by , 6 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Thank you for the report, I'm struggling to understand what's happening in your project. There's a number of migrations in there.
I don't think I have replicated the issue your seeing, what error do you see?
I can find the following possible issue that given an existing migrated model
when you change that model to
that generates a migration like
Which fails with
django.core.exceptions.FieldDoesNotExist: Foo has no field named 'order'
But actually I don't think that's what you're talking about here? Can you reduce it down a bit further?