[Django] #24225: KeyError when migrating in 1.8a1/master@728b6fd (does not occur in 1.7.3)
Django
------------------------------+--------------------
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
When running the `migrate` management command (even if there is nothing to
migrate) on 1.8a1 or the current GitHub master, I'm getting the following
uncaught `KeyError`:
{{{
Running migrations:
Rendering model states... DONE
Traceback (most recent call last):
File "manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File "d:\develop\myproj\django-
trunk\django\core\management\__init__.py", line 330, in
execute_from_command_line
utility.execute()
File "d:\develop\myproj\django-
trunk\django\core\management\__init__.py", line 322, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "d:\develop\myproj\django-trunk\django\core\management\base.py",
line 350, in run_from_argv
self.execute(*args, **cmd_options)
File "d:\develop\myproj\django-trunk\django\core\management\base.py",
line 401, in execute
output = self.handle(*args, **options)
File "d:\develop\myproj\django-
trunk\django\core\management\commands\migrate.py", line 187, in handle
executor.migrate(targets, plan, fake=options.get("fake", False))
File "d:\develop\myproj\django-trunk\django\db\migrations\executor.py",
line 89, in migrate
state = migration.mutate_state(state) # state is cloned inside
File "d:\develop\myproj\django-trunk\django\db\migrations\migration.py",
line 78, in mutate_state
operation.state_forwards(self.app_label, new_state)
File "d:\develop\myproj\django-
trunk\django\db\migrations\operations\fields.py", line 50, in
state_forwards
state.reload_model(app_label, self.model_name_lower)
File "d:\develop\myproj\django-trunk\django\db\migrations\state.py",
line 61, in reload_model
self._reload_one_model(rel_model._meta.app_label,
rel_model._meta.model_name)
File "d:\develop\myproj\django-trunk\django\db\migrations\state.py",
line 68, in _reload_one_model
self.models[app_label, model_name].render(self.apps)
KeyError: ('myapp', 'anoldmodel')
}}}
The reported model is created and then only appears twice in my
migrations:
In `myapp/migrations/0001_initial.py`:
{{{
migrations.AddField(
model_name='somemodel',
name='somefieldname',
field=models.ForeignKey(to='myapp.AValidModel'),
preserve_default=True,
),
}}}
and in `myapp/migrations/0002_a.py`:
{{{
migrations.RemoveField(
model_name='somemodel',
name='somefieldname',
),
}}}
Other than that, the model hasn't been changed in the migrations.
The application is closed source and I can't share real code with you
unfortunately.
I do have a debugger connected to the issue, but I have no idea where to
even start debugging this. By checking the stack frame for
`mutation.py:78` (`mutate_state()`), I can see that the "current
migration" when this occurs is a `0002_b.py`, i.e. another migration
dependent on `0001_initial.py` which therefore shouldn't be aware of the
field removal yet.
These migrations do work on 1.7.3. (But I can't really work on 1.7.3
because unit test startup takes minutes without `--keepdb`…)
Whatever the cause of this is, I highly recommend catching `KeyError` in
`_reload_one_model()` and rethrowing a proper exception with a descriptive
error message, including the stage and specific migration at which this
occurs. Because a `KeyError` being raised somewhere in the depths of the
migration system tells me nothing about what or where the issue is.
--
Ticket URL: <https://code.djangoproject.com/ticket/24225>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hheimbuerger):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
New description:
KeyError: ('myapp', 'somemodel')
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:1>
Django
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* version: master => 1.8alpha1
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Would you be able to reduce your set of models to reproduce this issue on
a minimal structure you would then be able to share?
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:2>
Django
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hheimbuerger):
Unfortunately, I don't. I extracted the suspected issue I described above
into a new project, but that works fine. So there must be one more factor
I'm missing to trigger the issue. Really reducing my actual big project
down to the cause would take me days, which I unfortunately don't have.
Besides, I wouldn't even know where to start.
I still can attach the debugger, so if you can tell me where to start
inspecting the state at the point in time the exception was raised, I can
do that.
Other than that, I now just removed those two aforementioned `AddField`
and `RemoveField` instructions from my old migrations (they are really
old, the two very first migrations—so while I'm sure editing old
migrations by hand is not a best practice, it shouldn't break anything in
this case) and that fixed it for my app.
So feel free to close as 'cannot reproduce'.
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:3>
Django
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hheimbuerger):
Claude inspired me to try again, and I think I managed to create a fairly
minimal reproduction case, which is now attached.
Run `python manage.py migrate`.
Result on Django 1.7.4:
{{{
Operations to perform:
Apply all migrations: admin, contenttypes, myapp, auth, sessions
Running migrations:
Applying contenttypes.0001_initial... OK
Applying auth.0001_initial... OK
Applying admin.0001_initial... OK
Applying myapp.0001_initial... OK
Applying myapp.0002_a... OK
Applying myapp.0003_a... OK
Applying myapp.0004_a... OK
Applying myapp.0002_b... OK
Applying myapp.0005_merge_a_and_b... OK
Applying sessions.0001_initial... OK
}}}
Result on Django master (from Jan 27):
{{{
Operations to perform:
Apply all migrations: admin, contenttypes, myapp, auth, sessions
Running migrations:
Rendering model states... DONE
Traceback (most recent call last):
File "manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File "d:\develop\django-trunk\django\core\management\__init__.py", line
330, in execute_from_command_line
utility.execute()
File "d:\develop\django-trunk\django\core\management\__init__.py", line
322, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "d:\develop\django-trunk\django\core\management\base.py", line 350,
in run_from_argv
self.execute(*args, **cmd_options)
File "d:\develop\django-trunk\django\core\management\base.py", line 401,
in execute
output = self.handle(*args, **options)
File "d:\develop\django-
trunk\django\core\management\commands\migrate.py", line 187, in handle
executor.migrate(targets, plan, fake=options.get("fake", False))
File "d:\develop\django-trunk\django\db\migrations\executor.py", line
89, in migrate
state = migration.mutate_state(state) # state is cloned inside
File "d:\develop\django-trunk\django\db\migrations\migration.py", line
78, in mutate_state
operation.state_forwards(self.app_label, new_state)
File "d:\develop\django-
trunk\django\db\migrations\operations\fields.py", line 50, in
state_forwards
state.reload_model(app_label, self.model_name_lower)
File "d:\develop\django-trunk\django\db\migrations\state.py", line 61,
in reload_model
self._reload_one_model(rel_model._meta.app_label,
rel_model._meta.model_name)
File "d:\develop\django-trunk\django\db\migrations\state.py", line 68,
in _reload_one_model
self.models[app_label, model_name].render(self.apps)
KeyError: ('myapp', 'somemodel')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:4>
Django
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hheimbuerger):
My biggest mysteries:
a) Why does removing the last operation from 0002_b.py 'unbreak' the
issue?
b) Why does taking the (entirely empty!) operations 0003 and 0004 out of
the loop 'unbreak' the issue?
(I.e. removing 0003 and 0004 and making 0005 dependent on 0002_a
directly.)
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:5>
Django
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Many thanks for the sample project which indeed triggers this bug.
The problem is that there are still leftover references of
'somemodel.account' foreign key on the `Account` model state (in
`_meta.related_objects`) even after it has been deleted. I think the
delete/remove operations do not clean enough metadata on related models.
Investigation continues...
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:6>
Django
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* has_patch: 0 => 1
Comment:
Henrik, could you try the patch to see if it solves the issue for your
full application?
https://github.com/django/django/pull/4062
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:7>
Django
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hheimbuerger):
Thanks! I'm on my way out now, but I'll give it a try tomorrow.
Cheers!
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:8>
Django
Reporter: hheimbuerger | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hheimbuerger):
I know you've already pulled back this patch because there are other
aspects to consider, but for the record:
This patch would have fixed the specific issue in my (closed source) real
world test case which originally triggered this bug report.
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:9>
Django
Reporter: hheimbuerger | Owner: MarkusH
Type: Bug | Status: assigned
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* owner: nobody => MarkusH
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:10>
Django
Reporter: hheimbuerger | Owner: MarkusH
Type: Bug | Status: assigned
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
Based on Claude's initial patch I started working on bigger migration
issue regarding states. More on the PR
https://github.com/django/django/pull/4097
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:11>
Django
Reporter: hheimbuerger | Owner: MarkusH
Type: Bug | Status: assigned
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:12>
Django
Reporter: hheimbuerger | Owner: MarkusH
Type: Bug | Status: assigned
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:13>
Django
Reporter: hheimbuerger | Owner: MarkusH
Type: Bug | Status: assigned
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"58d0dd9260156067263ea7a2da2685c3cd88e18a"]:
{{{
#!CommitTicketReference repository=""
revision="58d0dd9260156067263ea7a2da2685c3cd88e18a"
Refs #24225 -- Added failing test case for removing a previously added
field in migrations
When a related field is deleted, the related model must be updated. As
unchanged models are shared in migration states, the related model must
be re-rendered so that the change applies to a new copy of the related
model.
Thanks Henrik Heimbuerger for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:14>
Django
Reporter: hheimbuerger | Owner: MarkusH
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markus Holtermann <info@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b29f3b51204d53c1c8745966476543d068c173a2"]:
{{{
#!CommitTicketReference repository=""
revision="b29f3b51204d53c1c8745966476543d068c173a2"
Fixed #24225, #24264, #24282 -- Rewrote model reloading in migration
project state
Instead of naively reloading only directly related models (FK, O2O, M2M
relationship) the project state needs to reload their relations as well
as the model changes as well. Furthermore inheriting models (and super
models) need to be reloaded in order to keep inherited fields in sync.
To prevent endless recursive calls an iterative approach is taken.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:15>
Django
Reporter: hheimbuerger | Owner: MarkusH
Type: Bug | Status: closed
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"10ea9ef012cc11c9ae67ab4ba5d84e0d5d192732"]:
{{{
#!CommitTicketReference repository=""
revision="10ea9ef012cc11c9ae67ab4ba5d84e0d5d192732"
[1.8.x] Refs #24225 -- Added failing test case for removing a previously
added field in migrations
When a related field is deleted, the related model must be updated. As
unchanged models are shared in migration states, the related model must
be re-rendered so that the change applies to a new copy of the related
model.
Thanks Henrik Heimbuerger for the report.
Backport of 58d0dd9260156067263ea7a2da2685c3cd88e18a from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:16>
Django
Reporter: hheimbuerger | Owner: MarkusH
Type: Bug | Status: closed
Component: Migrations | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"a1ba4627931591b80afa46e38e261f354151d91a"]:
{{{
#!CommitTicketReference repository=""
revision="a1ba4627931591b80afa46e38e261f354151d91a"
[1.8.x] Fixed #24225, #24264, #24282 -- Rewrote model reloading in
migration project state
Instead of naively reloading only directly related models (FK, O2O, M2M
relationship) the project state needs to reload their relations as well
as the model changes as well. Furthermore inheriting models (and super
models) need to be reloaded in order to keep inherited fields in sync.
To prevent endless recursive calls an iterative approach is taken.
Backport of b29f3b51204d53c1c8745966476543d068c173a2 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24225#comment:17>