[Django] #33449: Migration autodetector fails for models with field named _order, but not using Meta.order_with_respect_to
Django
using Meta.order_with_respect_to
------------------------------------------+------------------------
Reporter: Fabian Büchler | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 4.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
The commit
[aa4acc1|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`:
{{{#!python
$ ./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:
{{{#!python
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]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33449>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
using Meta.order_with_respect_to
Reporter: Fabian Büchler | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 4.0
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Fabian Büchler:
Old description:
New description:
The commit
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:1>
Django
using order_with_respect_to.
---------------------------------+------------------------------------
Reporter: Fabian Büchler | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 4.0
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: David Wobrock (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for the report! Would you like to prepare a patch? (a regression
test is required.)
Regression in aa4acc164d1247c0de515c959f7b09648b57dc42.
Reproduced at dc8bb35e39388d41b1f38b6c5d0181224e075f16.
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:2>
Django
using order_with_respect_to.
---------------------------------+------------------------------------
Reporter: Fabian Büchler | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 4.0
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Fabian Büchler):
I could do that, David. 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
?
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:3>
Django
using order_with_respect_to.
-------------------------------------+-------------------------------------
Reporter: Fabian Büchler | Owner: Fabian
| Büchler
Type: Bug | Status: assigned
Component: Migrations | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Fabian Büchler):
* owner: nobody => Fabian Büchler
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:4>
Django
using order_with_respect_to.
-------------------------------------+-------------------------------------
Reporter: Fabian Büchler | Owner: Fabian
| Büchler
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Fabian Büchler):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:5>
Django
using order_with_respect_to.
-------------------------------------+-------------------------------------
Reporter: Fabian Büchler | Owner: Fabian
| Büchler
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Fabian Büchler):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/15339 PR] submitted. CCLA pending.
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:6>
Django
using order_with_respect_to.
-------------------------------------+-------------------------------------
Reporter: Fabian Büchler | Owner: Fabian
| Büchler
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
Patch LGTM once a release note is added.
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:7>
Django
using order_with_respect_to.
-------------------------------------+-------------------------------------
Reporter: Fabian Büchler | Owner: Fabian
| Büchler
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:8>
Django
using order_with_respect_to.
-------------------------------------+-------------------------------------
Reporter: Fabian Büchler | Owner: Fabian
| Büchler
Component: Migrations | Version: 4.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"eeff1787b0aa23016e4844c0f537d5093a95a356" eeff1787]:
{{{
#!CommitTicketReference repository=""
revision="eeff1787b0aa23016e4844c0f537d5093a95a356"
Fixed #33449 -- Fixed makemigrations crash on models without
Meta.order_with_respect_to but with _order field.
Regression in aa4acc164d1247c0de515c959f7b09648b57dc42.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:9>
Django
using order_with_respect_to.
-------------------------------------+-------------------------------------
Reporter: Fabian Büchler | Owner: Fabian
| Büchler
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b32080219ebaacbee73f8abd8a9ddc85387f95ed" b320802]:
{{{
#!CommitTicketReference repository=""
revision="b32080219ebaacbee73f8abd8a9ddc85387f95ed"
[4.0.x] Fixed #33449 -- Fixed makemigrations crash on models without
Meta.order_with_respect_to but with _order field.
Regression in aa4acc164d1247c0de515c959f7b09648b57dc42.
Backport of eeff1787b0aa23016e4844c0f537d5093a95a356 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33449#comment:10>