#33449 closed Bug (fixed)
Migration autodetector crashes on models with field named _order, but not using order_with_respect_to.
Reported by: | Fabian Büchler | Owned by: | Fabian Büchler |
---|---|---|---|
Component: | Migrations | Version: | 4.0 |
Severity: | Release blocker | Keywords: | |
Cc: | David Wobrock | 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 (last modified by )
The commit https://github.com/django/django/commit/aa4acc164d1247c0de515c959f7b09648b57dc42 introduced a new function ModelState.get_field
in django.db.migrations.state
.
This converts the field name _order
to the one defined in options['order_with_respect_to']
automatically, which fails if the model has a field _order
but isn't using Meta.order_with_respect_to
.
That is the case for models generated by django-simple-history (https://github.com/jazzband/django-simple-history) for models that are originally using Meta.order_with_respect_to
: the resulting historical records model has only _order
but is not using the Meta option.
This shows when running mange.py migrate
or manage.py makemigrations
:
$ ./manage.py makemigrations --dry-run Waiting for port 'mysql:3306' timeout 1s (attempt 1/60) Port 'mysql:3306' is open Traceback (most recent call last): File "./manage.py", line 42, in <module> main() File "./manage.py", line 36, in main execute_from_command_line(sys.argv) File "/usr/local/lib/python3.8/site-packages/django/core/management/__init__.py", line 425, in execute_from_command_line utility.execute() File "/usr/local/lib/python3.8/site-packages/django/core/management/__init__.py", line 419, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "/usr/local/lib/python3.8/site-packages/django/core/management/base.py", line 373, in run_from_argv self.execute(*args, **cmd_options) File "/usr/local/lib/python3.8/site-packages/django/core/management/base.py", line 417, in execute output = self.handle(*args, **options) File "/usr/local/lib/python3.8/site-packages/django/core/management/base.py", line 90, in wrapped res = handle_func(*args, **kwargs) File "/usr/local/lib/python3.8/site-packages/django/core/management/commands/makemigrations.py", line 172, in handle changes = autodetector.changes( File "/usr/local/lib/python3.8/site-packages/django/db/migrations/autodetector.py", line 43, in changes changes = self._detect_changes(convert_apps, graph) File "/usr/local/lib/python3.8/site-packages/django/db/migrations/autodetector.py", line 189, in _detect_changes self.generate_altered_fields() File "/usr/local/lib/python3.8/site-packages/django/db/migrations/autodetector.py", line 928, in generate_altered_fields old_field = self.from_state.models[app_label, old_model_name].get_field(old_field_name) File "/usr/local/lib/python3.8/site-packages/django/db/migrations/state.py", line 689, in get_field self.options['order_with_respect_to'] KeyError: 'order_with_respect_to'
I believe this could be solved using a bit more defensive code, like:
def get_field(self, field_name): if field_name == '_order' and 'order_with_respect_to' in self.options: field_name = self.options['order_with_respect_to'] return self.fields[field_name]
Change History (10)
comment:1 by , 3 years ago
Description: | modified (diff) |
---|
comment:2 by , 3 years ago
Cc: | added |
---|---|
Severity: | Normal → Release blocker |
Summary: | Migration autodetector fails for models with field named _order, but not using Meta.order_with_respect_to → Migration autodetector crashes on models with field named _order, but not using order_with_respect_to. |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 3 years ago
I could do that, Mariusz. Do you have any suggestions regarding where to put the regression test, though? I checked before filing the issue and was a bit lost in the plentora of tests. Would it fit with the migration autodetector tests at https://github.com/django/django/blob/main/tests/migrations/test_autodetector.py ?
comment:4 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 3 years ago
I've got a fix including regression tests prepared, just need to check with my employer regarding ICLA or CCLA before submitting a PR.
https://github.com/fabianbuechler/django/commit/b74ded041b17f7ec4386e87a0f84a097a8d6df4d
comment:8 by , 3 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Thanks for the report! Would you like to prepare a patch? (a regression test is required.)
Regression in aa4acc164d1247c0de515c959f7b09648b57dc42.
Reproduced at dc8bb35e39388d41b1f38b6c5d0181224e075f16.