[Django] #28056: Reverse migration for model rename with cross-app ForeignKey fails
Django
----------------------------------------+------------------------
Reporter: Paul Tiplady | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.10
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 |
----------------------------------------+------------------------
When attempting to reverse a migration for the rename of a model, where
the model has ForeignKey references to it from models in other apps, the
reverse migration fails with the following error:
`ValueError: The field <foreignkey_field> was declared with a lazy
reference to '<old_model_name>', but app 'one' doesn't provide model
'<old_model_name>'`
I've put a simple repro up on github here: https://github.com/paultiplady
/django-migration-bug, but here's an outline of the problem:
With two apps, `one` and `two`, and the models:
one/models.py:
{{{
from django.db import models
class Rename1(models.Model):
pass
}}}
two/models.py:
{{{
from django.db import models
class Related2(models.Model):
rename1 = models.ForeignKey('one.Rename1')
}}}
Renaming Rename1 to Rename2 produces a simple migration:
0002_auto_20170408_1617.py:
{{{
class Migration(migrations.Migration):
dependencies = [
('one', '0001_initial'),
]
operations = [
migrations.RenameModel(
old_name='Rename1',
new_name='Rename2',
),
]
}}}
But when this migration is reversed, the error is raised:
`ValueError: The field two.Related2.rename1 was declared with a lazy
reference to 'one.rename1', but app 'one' doesn't provide model
'rename1'.`
It's unclear from the error message how I'd even go about attempting to
address this problem, and the docs don't mention anything about handling
this case (as far as I can tell).
Expected behaviour is to be able to handle this reverse-rename seamlessly,
or at least provide a sensible error message if this operation is
unsupported.
Naively it looks like the problem is that the `two.Related2` model has not
been given its own migration when its ForeignKey was changed from
`one.Rename1` to `one.Rename2`, instead that change is tracked implicitly
and incorrectly in the RenameModel step.
--
Ticket URL: <https://code.djangoproject.com/ticket/28056>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Paul Tiplady | Owner: Gaurav Sehgal
Type: Bug | Status: assigned
Component: Migrations | Version: 1.10
Severity: Normal | Resolution:
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 Gaurav Sehgal):
* owner: nobody => Gaurav Sehgal
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:1>
Django
Reporter: Paul Tiplady | Owner: Gaurav Sehgal
Type: Bug | Status: assigned
Severity: Normal | Resolution:
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 Tim Graham):
* stage: Unreviewed => Accepted
Comment:
I think the problem is in `ForeignKey.db_type()`. It uses
`self.target_field` which is using the state of the current model files
rather than migration's state.
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:2>
Django
Reporter: Paul Tiplady | Owner: Gaurav Sehgal
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Gaurav Sehgal):
Can't we have a different migration for rename1 as alterfield, so that its
migration's state handled properly.
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:3>
Django
Reporter: Paul Tiplady | Owner: Gaurav Sehgal
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Gaurav Sehgal):
* has_patch: 0 => 1
Comment:
PR - https://github.com/django/django/pull/8488
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:4>
Django
Reporter: Paul Tiplady | Owner: (none)
Type: Bug | Status: new
Component: Migrations | Version: 1.10
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Gaurav Sehgal):
* status: assigned => new
* owner: Gaurav Sehgal => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:5>
Django
Reporter: Paul Tiplady | Owner: (none)
Component: Migrations | Version: 1.10
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
Comment:
As described on the pull request, based on my testing, the patch doesn't
solve the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:6>
Django
Reporter: Paul Tiplady | Owner: Gaurav Sehgal
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Gaurav Sehgal):
* owner: (none) => Gaurav Sehgal
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:7>
Django
Reporter: Paul Tiplady | Owner: (none)
Component: Migrations | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Friedrich Delgado):
* cc: Friedrich Delgado (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:7>
Django
Reporter: Paul Tiplady | Owner: (none)
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Bhuvnesh):
I think this issue was solved in
[https://github.com/django/django/pull/12688/commits/aa4acc164d1247c0de515c959f7b09648b57dc42
aa4acc164d1247c0de515c959f7b09648b57dc42] and can be closed now.
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:8>
Django
Reporter: Paul Tiplady | Owner: Bhuvnesh
Type: Bug | Status: assigned
Component: Migrations | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Bhuvnesh):
* owner: (none) => Bhuvnesh
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:9>
Django
Reporter: Paul Tiplady | Owner: Bhuvnesh
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Bhuvnesh):
There seems to be some problem with this ticket, I wasn't able to
reproduce with django v1.10.7 as described above. I played with the sample
repo provided and observed that the error :
{{{ValueError: The field two.Related2.rename1 was declared with a lazy
reference to 'one.rename1', but app 'one' doesn't provide model
'rename1'.}}}
is produced when we run makemigrations command again after the RenameModel
migration file is generated. I think at that time django didn't had the
functionality to rename the cross-app referenced models, we can solve this
by editing the migration file two.0001_initial and set the fk to
"one.rename2" - removes the ValueError .
Another error:
{{{ValueError: Related model 'one.Rename1' cannot be resolved}}}
is thrown on migrating which is removed if we add a field instead of
"pass" inside {{{one.rename1}}} model.
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:10>
Django
Reporter: Paul Tiplady | Owner: Bhuvnesh
Component: Migrations | Version: 1.10
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => duplicate
Comment:
Good catch! `('two', '0001_initial')` dependency was missing in migration
generated in Django < 2.0,
Duplicate of #28493, fixed by 0891503fad458e7dfabc5adbf314f80e78e63f14.
--
Ticket URL: <https://code.djangoproject.com/ticket/28056#comment:11>