[Django] #24513: "... has no field named None" with M2MField when migrating
Django
-------------------------------+-----------------------------
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Severity: Normal | Keywords: migrations, bug
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------
In Django 1.8c1 migrations from scratch don't work for me anymore, this
effectively prevents testing.
(When you see project I'm talking about an app, btw)
See file traceback.txt for a test-call and a backtrace.
See file 0011_auto_20140914_2032.py for the migration that failed.
And see file user_model.py for the model that should be migrated.
---
I can't provide much code sadly. I tried to simplify my big project, but I
can't get it done without disclosing half of my product (or it would take
me days to do so …).
Instead I did a git-bisect to get the commit that makes my migrations
fail:
{{{
(testenv)kondou:django/ (4e9ecfe) $ git bisect good
a1ba4627931591b80afa46e38e261f354151d91a is the first bad commit
commit a1ba4627931591b80afa46e38e261f354151d91a
Author: Markus Holtermann <in...@markusholtermann.eu>
Date: Mon Feb 9 01:11:25 2015 +0100
[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
:040000 040000 2faa54b906f309ddb353a73abbef7db950b18d1c
2eb05be062be72e8a8f4d54961fbe3ec96d0edf1 M django
}}}
Doing a git revert a1ba4627931591b80afa46e38e261f354151d91a on 1.8c1 tag
indeed fixes this problem for me.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by Kondou-ger):
* Attachment "traceback.txt" added.
Traceback
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by Kondou-ger):
* Attachment "0011_auto_20140914_2032.py" added.
Migration
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
Changes (by Kondou-ger):
* Attachment "user_model.py" added.
Model that should be migrated
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Can you to backtrack the exception and try to figure out why the
`get_field(None)` call is happening?
{{{
File "/home/kondou/testenv/lib/python3.4/site-
packages/django/db/models/options.py", line 552, in get_field
return self.fields_map[field_name]
KeyError: None
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:1>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by Kondou-ger):
In
[https://github.com/django/django/blob/1.8c1/django/db/backends/base/schema.py#L268
django.db.backends.base.schema.py#L268] the call to `get_field()` is done,
where
{{{
fields==(None, 'group'),
model._meta.unique_together==((None, 'group'),),
model==project.user__groups
}}}
The call to this is done at
[https://github.com/django/django/blob/1.8c1/django/db/backends/sqlite3/schema.py#L137
django.db.backends.sqlite3.schema.py#L137] where `temp_model==<class
'__fake__.User_groups'>`
I also have a Groups model that refers to User. But there is no unique
together constraint in the whole file:
{{{
class Groups(models.Model):
owner = models.ForeignKey(settings.AUTH_USER_MODEL,
related_name='owner')
members = models.ManyToManyField(settings.AUTH_USER_MODEL,
related_name='members')
name = models.CharField(max_length=50, unique=True,
validators=[RegexValidator(regex=r'^[\w.@+-]+$')])
displayname = models.CharField(max_length=150, blank=True)
description = models.CharField(max_length=5000, blank=True)
default_moderators = models.ManyToManyField(settings.AUTH_USER_MODEL,
related_name="group_default_moderators")
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:2>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by kaedroho):
I've so far been unable to reproduce this issue. It looks like it's
failing when running that AlterField operation on the "groups" field.
Could you paste the code for the user model before this migration so we
can see what this operation is changing?
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:3>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Changes (by Kondou-ger):
* Attachment "diff.txt" added.
Diff that created migration 0011...
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by Kondou-ger):
I attached the diff that created the migration.
Weird thing is, I can't reproduce this either. If I start a fresh app with
all my models in I get a different migration than a squashed on from my
app that produces the bug. Using this created initial migration as a
"squashed" migration even works as a workaround …
I'm doing a git-bisect on my project with django1.8c1 now, to see what
exactly introduced the bug in my project.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:4>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by Kondou-ger):
The git-bisect on my project confirmed that the combination on migration
0011... and diff.txt introduced the bug for me.
Another strange sidenote: This bug may be caching related … Sometimes
(~80%) I get it, sometimes I don't …
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:5>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.8rc1
Keywords: migrations, bug | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
I think the unique_together constraint is part of the m2m through table
which is automatically created by Django (which is `<class
'__fake__.User_groups'>`). The odd thing, the field `None` should be
`user` (FK from the m2m through table to your user model table). I have no
idea yet why or where the name resolution fails / the field cache isn't
cleared.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:6>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8rc1
Severity: Normal | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
Comment:
I guess we can tentatively accept the ticket, but the inability to
reproduce it reliably may make solving it somewhat difficult.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:7>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8rc1
Severity: Normal | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Kondou-ger):
Today I ran into this error again, while changing the `related_name` of a
M2M to self with `symmetrical=False`, and could finally create a project
that is able to reproduce this.
You can reproduce it by just running `./manage.py test`. Be minded though
that (about 50% now) sometimes the migration actually works and no
exception is thrown (probably caching related).
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:8>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8rc1
Severity: Normal | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Kondou-ger):
* Attachment "testproject.zip" added.
Project that reproduces this error. Do ./manage.py test a few times
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8rc1
Severity: Normal | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by patrys):
I believe I have tracked it down further:
In `django/db/backends/sqlite3/schema.py` line 234 (`_alter_many_to_many`)
we have:
{{{
override_uniques=(new_field.m2m_field_name(),
new_field.m2m_reverse_field_name())
}}}
However `new_field.m2m_field_name()` return `None`.
`new_field.m2m_field_name()` is a partial that evaluates to
`self._get_m2m_attr(related, 'name')` where `related` is a class the
relation is pointing to.
The implementation lives in `django/db/models/fields/related.py` line 2470
(`_get_m2m_attr`):
{{{
for f in self.rel.through._meta.fields:
if (f.is_relation and f.rel.to == related.related_model and
(link_field_name is None or link_field_name == f.name)):
}}}
However at this point `f.rel.to` and `related` point to two different
classes that are not considered equal. This could be caused by the fact
that the operation is an `AlterField` and it creates two states of the
affected class—one before and one after the migration—even though all of
the changes happen in a separate m2m model.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:9>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8rc1
Severity: Normal | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by patrys):
Not sure whether I'm on the right path but in
`django/db/migrations/state.py` in `ProjectState.reload_model` the
following happens:
{{{
try:
old_model = self.apps.get_model(app_label, model_name)
except LookupError:
related_models = set()
else:
# Get all relations to and from the old model before reloading,
# as _meta.apps may change
related_models = get_related_models_recursive(old_model)
}}}
The resulting set correctly contains all related m2m models. Later however
this code is run:
{{{
for rel_app_label, rel_model_name in related_models:
try:
model_state = self.models[rel_app_label, rel_model_name]
except KeyError:
pass
else:
states_to_be_rendered.append(model_state)
}}}
This lookup fails for all related m2m models which causes them not to
appear in the to-be-rerendered state list. This in turn results in the m2m
models pointing to the old (previously rendered) version of the altered
class.
I can trigger this problem quite reliably by having the migration first
touch any non-m2m field of the affected class tree (which would cause all
of the related models to be rendered) and then alter an m2m field (which
fails m2m name lookups due to the relations pointing to out of date
models).
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:10>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8rc1
Severity: Normal | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by patrys):
Side note:
Tracing the cause of this became much easier after I've changed
`_get_m2m_attr()` end with a `raise AttributeError()`. There does not seem
to be a case when it silently returning `None` in case of a missing
attribute match is deliberate and expected.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:11>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8rc1
Severity: Normal | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by apollo13):
* cc: apollo13 (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:12>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8rc1
Severity: Normal | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by patrys):
I have found the reason of the crash.
It turns that if a model has a relation to itself,
`ProjectState.reload_model` will place it twice on the
`states_to_be_rendered` list. This causes all relations to point to the
registered version of the model (and later cause problems where they are
unable to find their reverse relationship which leads to the field name
being set to `None`).
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:13>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* version: 1.8rc1 => 1.8
* severity: Normal => Release blocker
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:14>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by patrys):
Pull request: https://github.com/django/django/pull/4450
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:15>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_docs: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:16>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Wtower):
Hi, FYI I have been experiencing the same issue. I was wondering if it
would be helpful to let you know of more details in here, open a new
ticket or test the patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:17>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
Hey Wtower, if you can try out [https://github.com/django/django/pull/4450
patrys' patch] and leave a comment on the pull request with your results
it'd be much appreciated.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:18>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by patrys):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:19>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Wtower):
The patch seems to work fine for me, thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:20>
Django
---------------------------------+------------------------------------
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by Wtower):
I was eager to speak, I'm afraid. For some reason the tests do not seem to
fail every time. Seems so strange. I got all details if interested.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:21>
Django
---------------------------------+------------------------------------
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by MarkusH):
You're saying the patch doesn't fully work? In that case, can you check
"Patch needs improvement" and "Needs tests" on this ticket here and share
your details, please
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:22>
Django
---------------------------------+------------------------------------
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:23>
Django
---------------------------------+------------------------------------
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
Keywords: migrations, bug | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Comment (by Wtower):
The second patch seems to consistently work even after performing several
different tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:24>
Django
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:25>
Django
-------------------------------------+-------------------------------------
Reporter: Kondou-ger | Owner: nobody
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: migrations, bug | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"0385dad073270c37f8c4a5f13edce43f2a69ba8a" 0385dad]:
{{{
#!CommitTicketReference repository=""
revision="0385dad073270c37f8c4a5f13edce43f2a69ba8a"
Fixed #24513 -- Made sure a model is only rendered once during reloads
This also prevents state modifications from corrupting previous states.
Previously, when a model defining a relation was unregistered first,
clearing the cache would cause its related models' _meta to be cleared
and would result in the old models losing track of their relations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:26>
Django
-------------------------------------+-------------------------------------
Reporter: Kondou-ger | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.8
Severity: Release blocker | Resolution: fixed
Keywords: migrations, bug | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Markus Holtermann <info@…>):
In [changeset:"9f632dc702a1b8b4c8044fb34bb3d5f2445dd4ee" 9f632dc]:
{{{
#!CommitTicketReference repository=""
revision="9f632dc702a1b8b4c8044fb34bb3d5f2445dd4ee"
[1.8.x] Fixed #24513 -- Made sure a model is only rendered once during
reloads
This also prevents state modifications from corrupting previous states.
Previously, when a model defining a relation was unregistered first,
clearing the cache would cause its related models' _meta to be cleared
and would result in the old models losing track of their relations.
Backport of 0385dad073270c37f8c4a5f13edce43f2a69ba8a from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24513#comment:27>