#24513 closed Bug (fixed)
"... has no field named None" with M2MField when migrating
Reported by: | Christopher Schäpers | Owned by: | nobody |
---|---|---|---|
Component: | Migrations | Version: | 1.8 |
Severity: | Release blocker | Keywords: | migrations, bug |
Cc: | Florian Apolloner | 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
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 <info@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.
Attachments (5)
Change History (32)
by , 10 years ago
Attachment: | traceback.txt added |
---|
comment:1 by , 10 years ago
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
comment:2 by , 10 years ago
In 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 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")
Edit: All this info is included in traceback though … Have a look at the part under KeyError: None
…
comment:3 by , 10 years ago
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?
comment:4 by , 10 years ago
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.
comment:5 by , 10 years ago
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 …
comment:6 by , 10 years ago
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.
comment:7 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Bug |
I guess we can tentatively accept the ticket, but the inability to reproduce it reliably may make solving it somewhat difficult.
comment:8 by , 10 years ago
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).
Edit: I probably get it that often, because my User model has 4 self, symmetrical=False
entries.
by , 10 years ago
Attachment: | testproject.zip added |
---|
Project that reproduces this error. Do ./manage.py test a few times
comment:9 by , 10 years ago
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()
returns 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.
comment:10 by , 10 years ago
Not sure whether I'm on the right track 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).
comment:11 by , 10 years ago
Side note:
Tracing the cause of this became much easier after I've changed _get_m2m_attr()
to 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.
comment:12 by , 10 years ago
Cc: | added |
---|
comment:13 by , 10 years ago
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
).
comment:14 by , 10 years ago
Severity: | Normal → Release blocker |
---|---|
Version: | 1.8rc1 → 1.8 |
comment:16 by , 10 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Needs tests: | set |
comment:17 by , 10 years ago
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.
comment:18 by , 10 years ago
Hey Wtower, if you can try out patrys' patch and leave a comment on the pull request with your results it'd be much appreciated.
comment:19 by , 10 years ago
Needs tests: | unset |
---|
comment:21 by , 10 years ago
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.
comment:22 by , 10 years ago
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
comment:23 by , 10 years ago
Patch needs improvement: | set |
---|
comment:24 by , 10 years ago
The second patch seems to consistently work even after performing several different tests.
comment:25 by , 10 years ago
Needs documentation: | unset |
---|---|
Patch needs improvement: | unset |
Triage Stage: | Accepted → Ready for checkin |
Traceback