[Django] #24100: Make `post_migrate` dispatch the migration plan.
Django
---------------------------------------+---------------------------
Reporter: charettes | Owner: charettes
Type: New feature | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+---------------------------
Accepting based on +1's received from Aymeric and Claude in order to solve
#24067, #24075.
I should have a patch with doc ready by the end of the day.
--
Ticket URL: <https://code.djangoproject.com/ticket/24100>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: charettes | Owner: charettes
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* has_patch: 0 => 1
Comment:
Created a [https://github.com/django/django/pull/3866 PR] for this ticket.
I'd like to give a try at fixing #24067 before getting this merge to make
sure the provided `plan` is sufficient to issue `ContentType` renames.
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:1>
Django
Reporter: charettes | Owner: charettes
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* cc: info+coding@… (added)
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* needs_docs: 0 => 1
Comment:
As already mentioned on the PR, I'm not sure the current API is stable
enough (in terms of "Django doesn't crash", not in terms of "We change a
function signature") for public usage. I'd like to have the alpha and beta
for tests. Tim noted on IRC that changing the API after beta isn't really
a thing we should do which I do understand.
A workaround we could also simply backport to 1.7 (and add `plan` and
`state` in 1.9), is a check for the respective first migrations being
applied:
In `django.contrib.contenttypes.management`:
{{{#!python
def update_contenttypes(...):
from django.db.migration.loader import MigrationLoader
loader = MigrationLoader(connection)
loader.load_disk()
if not ('contenttypes', '0001_initial') in loader.applied_migrations:
return
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:2>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Changes (by MarkusH):
* owner: charettes => MarkusH
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:3>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by MarkusH):
PR: https://github.com/django/django/pull/4405
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:4>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Changes (by frnhr):
* cc: frnhr (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:5>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Changes (by bronger):
* cc: bronger@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:6>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* cc: info+coding@… (removed)
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:7>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
See #25931 for a related issue with `ContentType` creation and backwards
migrations.
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:8>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by amosonn):
For issue #25931, it is sufficient to use the project_state after the
migration, and no need to review the plan. This might make it an easier
pick for testing that part of the interface.
As for the `ContentType` renames, I think that perhaps it would be cleaner
to add signals for specific migration operations (such as `RenameModel`),
which get passed the two states of the migration and perform their
additional action (here, renaming the relevant `ContentType` entry), and
have `contrib.contenttypes` connect to that - rather than having it sift
through the whole plan itself. This way the change can also be done mid-
migration, and enable the rest of the migration to rely on a consistent
`ContentType` table.
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:9>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
> As for the ContentType renames, I think that perhaps it would be cleaner
to add signals for specific migration operations (such as RenameModel),
which get passed the two states of the migration and perform their
additional action (here, renaming the relevant ContentType entry), and
have contrib.contenttypes connect to that - rather than having it sift
through the whole plan itself. This way the change can also be done mid-
migration, and enable the rest of the migration to rely on a consistent
ContentType table.
The down side of basing the detection on `RenameModel` only is the fact we
don't handle any third party operation or usage of
`SeparateDatabaseAndState` that could also alter the model state.
I agree with you that `contrib.contenttypes` shouldn't have to figure out
which model were renamed from the plan by itself. Defining a ''utility''
function in the `migrations` module and using it the signal receiver would
make more sense.
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:10>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_tests: 1 => 0
Comment:
[https://github.com/django/django/pull/5835 PR].
@amosonn, I ended up going with an approach similar to what you proposed.
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:11>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | 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 timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:12>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
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 timgraham):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:13>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
This may also fix #26588.
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:14>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/6586 Updated PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:15>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
Old description:
> Accepting based on +1's received from Aymeric and Claude in order to
> solve #24067, #24075.
>
> I should have a patch with doc ready by the end of the day.
New description:
Accepting based on +1's received from Aymeric and Claude in order to solve
#24067, #24075.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:16>
Django
Reporter: charettes | Owner: MarkusH
Component: Migrations | Version: master
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 Simon Charette <charette.s@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"f937c9ec975ebd719f0c22e5d1d5f5fb87ff1edd" f937c9e]:
{{{
#!CommitTicketReference repository=""
revision="f937c9ec975ebd719f0c22e5d1d5f5fb87ff1edd"
Fixed #24100 -- Made the migration signals dispatch its plan and apps.
Thanks Markus for your contribution and Tim for your review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:17>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | 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 Simon Charette <charette.s@…>):
In [changeset:"05a9f3a09a5a2334fe1d1675f46ea524240999b3" 05a9f3a0]:
{{{
#!CommitTicketReference repository=""
revision="05a9f3a09a5a2334fe1d1675f46ea524240999b3"
Refs #24100 -- Fixed a test failure on MySQL related to non-transactional
DDL.
Thanks Tim for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:18>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | 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 Simon Charette <charette.s@…>):
In [changeset:"d1757d8df486b689172d2584ded52fad916bcc33" d1757d8d]:
{{{
#!CommitTicketReference repository=""
revision="d1757d8df486b689172d2584ded52fad916bcc33"
Fixed #27044 -- Included already applied migration changes in the post-
migrate state when the execution plan is empty.
Refs #24100.
Thanks tkhyn for the report and Tim for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:19>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | 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 Simon Charette <charette.s@…>):
In [changeset:"d5c4ea524679a787fe11c927448e44e95646096b" d5c4ea52]:
{{{
#!CommitTicketReference repository=""
revision="d5c4ea524679a787fe11c927448e44e95646096b"
Fixed #27100 -- Included already applied migration changes in the pre-
migrate state.
Refs #24100.
Thanks Tim for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:20>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | 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 Simon Charette <charette.s@…>):
In [changeset:"0b454804db5729eedbb117c90de4a3ad5fb397ee" 0b45480]:
{{{
#!CommitTicketReference repository=""
revision="0b454804db5729eedbb117c90de4a3ad5fb397ee"
[1.10.x] Fixed #27100 -- Included already applied migration changes in the
pre-migrate state.
Refs #24100.
Thanks Tim for the review.
Backport of d5c4ea524679a787fe11c927448e44e95646096b from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:22>
Django
Reporter: charettes | Owner: MarkusH
Type: New feature | 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 Simon Charette <charette.s@…>):
In [changeset:"1c60765d6307cf5bb659d9157a990d7410108424" 1c60765d]:
{{{
#!CommitTicketReference repository=""
revision="1c60765d6307cf5bb659d9157a990d7410108424"
[1.10.x] Fixed #27044 -- Included already applied migration changes in the
post-migrate state when the execution plan is empty.
Refs #24100.
Thanks tkhyn for the report and Tim for the review.
Backport of d1757d8df486b689172d2584ded52fad916bcc33 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24100#comment:21>