#24100 closed New feature (fixed)
Make `post_migrate` dispatch the migration plan.
| 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 (22)
comment:2 by , 11 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.migration.loader import MigrationLoader loader = MigrationLoader(connection) loader.load_disk() if not ('contenttypes', '0001_initial') in loader.applied_migrations: return
comment:3 by , 11 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:5 by , 10 years ago
| Cc: | added |
|---|
comment:6 by , 10 years ago
| Cc: | added |
|---|
comment:7 by , 10 years ago
| Cc: | removed |
|---|---|
| Needs documentation: | unset |
| Patch needs improvement: | unset |
comment:8 by , 10 years ago
See #25931 for a related issue with ContentType creation and backwards migrations.
comment:9 by , 10 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 , 10 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 , 10 years ago
| Needs tests: | unset |
|---|
PR.
@amosonn, I ended up going with an approach similar to what you proposed.
comment:12 by , 10 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
comment:13 by , 10 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
planis sufficient to issueContentTyperenames.