Opened 10 years ago
Last modified 8 years ago
#24100 closed New feature
Make `post_migrate` dispatch the migration plan. — at Version 16
Reported by: | Simon Charette | Owned by: | Markus Holtermann |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | frnhr, bronger@… | 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 (last modified by )
Change History (16)
comment:2 by , 10 years ago
Cc: | added |
---|---|
Needs documentation: | set |
Needs tests: | set |
Patch needs improvement: | set |
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
:
def update_contenttypes(...): from django.db import connection from django.db.migrations.loader import MigrationLoader loader = MigrationLoader(connection) loader.load_disk() if not ('contenttypes', '0001_initial') in loader.applied_migrations: return
comment:3 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 9 years ago
Cc: | added |
---|
comment:6 by , 9 years ago
Cc: | added |
---|
comment:7 by , 9 years ago
Cc: | removed |
---|---|
Needs documentation: | unset |
Patch needs improvement: | unset |
comment:8 by , 9 years ago
See #25931 for a related issue with ContentType
creation and backwards migrations.
comment:9 by , 9 years ago
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.
comment:10 by , 9 years ago
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.
comment:11 by , 9 years ago
Needs tests: | unset |
---|
PR.
@amosonn, I ended up going with an approach similar to what you proposed.
comment:12 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:13 by , 9 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
comment:16 by , 9 years ago
Description: | modified (diff) |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Created a PR for this ticket.
I'd like to give a try at fixing #24067 before getting this merged to make sure the provided
plan
is sufficient to issueContentType
renames.