[Django] #24184: Migrate auto-fake behavior is not safe
Django
--------------------------------------+------------------------
Reporter: carljm | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
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 |
--------------------------------------+------------------------
Currently, `manage.py migrate` will auto-fake-apply initial migrations if
it sees that the tables already exist, but it doesn't do any checking that
the schema of those tables actually matches the schema that the migration
would have created. And our documentation doesn't call out the importance
of making sure that your existing schema matches the models you used to
generate the initial migrations.
In an ideal world, we might keep the current behavior and automatically
verify a full schema match before auto-faking, but I doubt that's feasible
and in any case would be a major new feature.
Short of that, I think the unsafe auto-fake behavior should not be the
default; it should require a flag like `--fake-if-exists` or something
(needs bikeshedding) to turn it on. Wherever that flag is
documented/demonstrated in the docs (e.g. in the
[[https://docs.djangoproject.com/en/1.7/topics/migrations/#upgrading-from-
south upgrade-from-South docs]]), we should be very clear that it is only
safe to use if you are sure that your schema is properly up-to-date with
your models used to generate your initial migration.
(This ticket was motivated by #24178.)
--
Ticket URL: <https://code.djangoproject.com/ticket/24184>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: carljm | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
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 MarkusH):
* cc: MarkusH (added)
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:1>
Django
Reporter: carljm | Owner: MarkusH
Type: Bug | Status: assigned
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* status: new => assigned
* owner: nobody => MarkusH
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:2>
Django
Reporter: carljm | Owner: MarkusH
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 MarkusH):
* needs_docs: 0 => 1
* has_patch: 0 => 1
Comment:
Initial PR: https://github.com/django/django/pull/3967
Should that be backported to 1.7 given that it's the first version you
upgrade to when you move away from South.
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:3>
Django
Reporter: carljm | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
As discussed in IRC, I don't think we should backport this to 1.7 (much as
I'd like to), since it's a backwards-incompatible change in user-facing
behavior. We'll just have to put a warning in the docs for 1.7.
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:4>
Django
Reporter: carljm | Owner: MarkusH
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 MarkusH):
* needs_docs: 1 => 0
Comment:
PR for master and 1.8 is ready.
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:5>
Django
Reporter: carljm | Owner: MarkusH
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 carljm):
Reviewed PR, looks good to me, just needs some doc tweaks and to be merged
up with latest master.
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:6>
Django
Reporter: carljm | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by carljm):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:7>
Django
Reporter: carljm | Owner: MarkusH
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markus Holtermann <info@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"f287bec5833d75750fa6368bc2802741b7924533"]:
{{{
#!CommitTicketReference repository=""
revision="f287bec5833d75750fa6368bc2802741b7924533"
Fixed #24184 -- Prevented automatic soft-apply of migrations
Previously Django only checked for the table name in CreateModel
operations in initial migrations and faked the migration automatically.
This led to various errors and unexpected behavior. The newly introduced
--fake-initial flag to the migrate command must be passed to get the
same behavior again. With this change Django will bail out in with a
"duplicate relation / table" error instead.
Thanks Carl Meyer and Tim Graham for the documentation update, report
and review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:8>
Django
Reporter: carljm | Owner: MarkusH
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"bd80fa6b0f7e5a0cc4ea26cedd56d0c4c4894420"]:
{{{
#!CommitTicketReference repository=""
revision="bd80fa6b0f7e5a0cc4ea26cedd56d0c4c4894420"
[1.8.x] Fixed #24184 -- Prevented automatic soft-apply of migrations
Previously Django only checked for the table name in CreateModel
operations in initial migrations and faked the migration automatically.
This led to various errors and unexpected behavior. The newly introduced
--fake-initial flag to the migrate command must be passed to get the
same behavior again. With this change Django will bail out in with a
"duplicate relation / table" error instead.
Thanks Carl Meyer and Tim Graham for the documentation update, report
and review.
Backport of f287bec5833d75750fa6368bc2802741b7924533 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:9>
Django
Reporter: carljm | Owner: MarkusH
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
I opened #24337 to track the documentation update for 1.7
--
Ticket URL: <https://code.djangoproject.com/ticket/24184#comment:10>