[Django] #29177: Unmanaged models with ForeignKeys do not get those fields serialized into their migration state when CreateModel happens.
Django
their migration state when CreateModel happens.
----------------------------------------+------------------------
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
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 |
----------------------------------------+------------------------
Given models like the following, where `B` is unmanaged, and `C` is a
normal concrete `Model`:
{{{
from django.db import models
class A(models.Model):
class Meta:
db_table = "test_a"
class B(models.Model):
a = models.ForeignKey(A, related_name="+", on_delete=models.CASCADE)
class Meta:
managed = False
db_table = "test_b"
class C(models.Model):
a = models.ForeignKey(A, related_name="+", on_delete=models.CASCADE)
class Meta:
db_table = "test_c"
}}}
when `python manage.py makemigrations <appname>` is called, `B` ends up
looking like:
{{{
migrations.CreateModel(
name='B',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True,
serialize=False, verbose_name='ID')),
],
options={
'db_table': 'test_b',
'managed': False,
},
)
}}}
whilst `C` correctly gets:
{{{
migrations.CreateModel(
name='C',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True,
serialize=False, verbose_name='ID')),
('a',
models.ForeignKey(on_delete=django.db.models.deletion.CASCADE,
related_name='+', to='whee.A')),
],
options={
'db_table': 'test_c',
},
)
}}}
Because `B` doesn't have the `a` attribute, any subsequent data migrations
which would like to query on it, like so:
{{{
def forwards(apps, schema_editor):
B = apps.get_model('whee', 'B')
B.objects.filter(a=1)
}}}
will crash.
With the assistance of Markus on IRC:
{{{
MarkusH: do you want to try what's gonna happen when you just drop the if-
condition in
https://github.com/django/django/blob/75527c0f8360ae3555fcf67ce19b4cb910b69b9d/django/db/migrations/autodetector.py#L570-L572
?
}}}
commenting out the lines
{{{
if not model_opts.managed:
continue
}}}
allows the autodetector to generate the following:
{{{
migrations.CreateModel(
name='A',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True,
serialize=False, verbose_name='ID')),
],
options={
'db_table': 'test_a',
},
),
migrations.AddField(
model_name='b',
name='a',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE,
related_name='+', to='whee.A'),
)
}}}
and subsequently the data migration can access `B.a`
If `A` itself is `managed=False` the migration generated is instead:
{{{
migrations.CreateModel(
name='B',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True,
serialize=False, verbose_name='ID')),
('a',
models.ForeignKey(on_delete=django.db.models.deletion.CASCADE,
related_name='+', to='whee.A')),
],
options={
'db_table': 'test_b',
'managed': False,
},
)
}}}
Running the test suite via `./runtests.py` as of commit
75527c0f8360ae3555fcf67ce19b4cb910b69b9d doesn't seem to cause any
failures, with those lines commented out.
Markus also pointed out commit
https://github.com/django/django/commit/215aa4f53b6bbd07d5c1eecfa94e7fcd00da813e
which references #23415
I noticed this "issue" while bringing a project up from 1.9, so it's not a
recent problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/29177>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | 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 Keryn Knight):
* cc: Keryn Knight (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:1>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | 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
Description changed by Keryn Knight:
Old description:
> if-condition in
New description:
{{{
from django.db import models
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:2>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
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 Simon Charette):
* stage: Unreviewed => Accepted
Comment:
Seems like a legitimate bug.
Do you think you could submit a PR with the removed lines and a test in
[https://github.com/django/django/blob/master/tests/migrations/test_autodetector.py
test_autodetector.py] for it?
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:3>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Keryn Knight):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* needs_docs: 0 => 1
Comment:
WIP PR: https://github.com/django/django/pull/9741
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:4>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by YBull):
Using version 2.0.5, I just encountered this bug and when trying to work
around it, discovered that it's a little more expansive than the example
model with A, B, C above shows. In my case, I had used the 'inspectdb'
command to generate models for 12 legacy tables (all with managed =
False). The inspectdb command did a reasonably good job and included all
the ForeignKey fields it should have. But even after I commented out the
lines in autodetector.py to work around this bug, I discovered that the
'makemigrations' command still has another related problem with Fkeys on
unmanaged models. Specifically, whenever a new unmanaged model is
referenced by another model's Fkeys, you must run 'makemigrations' for the
referenced table BEFORE adding the model(s) containing the reference. So
in my case, even though the models.py was valid and complete at the
beginning, I had to temporarily remove all the new dependent models to
carefully generate a sequence of six separate migrations to get all my 12
legacy tables in with the correct FKey declarations in the migration
scripts.
Being new to Django and the code base, I wanted to report this here, but
I'm not sure whether the fix for this aspect is actually a separate
related bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:5>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Sven R. Kunze):
Bug is also present on 1.11.
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:6>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Daniel Naab):
I hit this bug also, and worked around by:
1) Making the models `managed = True`
2) Running `manage.py makemigrations` for those models
3) Manually switched all `'managed': True` columns in the migration to
`'managed': False`
4) Switch models back to `managed = False`
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:7>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Daniel Naab):
* cc: Daniel Naab (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:8>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Sergey Yurchenko):
Unmanaged models should not work in migrations by their nature.
You can make direct import of this model in migrations and use it.
I think it is expected behaviour not a bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:9>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
Sergey,
> Unmanaged models should not work in migrations by their nature.
Is there a reason for that? How would the migration framework know how to
generate references from managed models to unmanaged ones then (e.g. a
foreign key)? The migration framework has supported managed model
references for a while and even
[https://docs.djangoproject.com/en/2.1/howto/writing-migrations/#changing-
an-unmanaged-model-to-managed documents how to move an unmanaged model to
a managed one].
> You can make direct import of this model in migrations and use it.
Direct model imports in migrations will break over time if the model
definition happen to change and won't work when mixed up with
state/migration models. The
[https://docs.djangoproject.com/en/2.1/topics/migrations/#historical-
models documentation warns about this pattern].
I still believe this is a legitimate bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:10>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Fabio Sangiovanni):
* cc: Fabio Sangiovanni (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:11>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Ponytech):
Replying to [comment:7 Daniel Naab]:
> I hit this bug also, and worked around by:
>
> 1) Making the models `managed = True`
> 2) Running `manage.py makemigrations` for those models
> 3) Manually switched all `'managed': True` models in the migration to
`'managed': False`
> 4) Switch models back to `managed = False`
Same bug here and this workaround saved my day.
Thanks a lot!
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:12>
Django
their migration state when CreateModel happens.
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by João Paulo Limberger - JOTAPE - Shoo):
Thanks! This workaround save my day Too!!!
> Replying to [comment:7 Daniel Naab]:
> > I hit this bug also, and worked around by:
> >
> > 1) Making the models `managed = True`
> > 2) Running `manage.py makemigrations` for those models
> > 3) Manually switched all `'managed': True` models in the migration to
`'managed': False`
> > 4) Switch models back to `managed = False`
>
> Same bug here and this workaround saved my day.
> Thanks a lot!
--
Ticket URL: <https://code.djangoproject.com/ticket/29177#comment:13>