chore(migrations): Improve migration guidance - #121907
Open
wedamija wants to merge 1 commit into
Open
Conversation
Be more specific about exactly how to remove columns and tables
vaind
approved these changes
Aug 13, 2026
| 2. Remove all code references | ||
| 3. Replace `RemoveField` with `SafeRemoveField(..., deletion_action=DeletionAction.MOVE_TO_PENDING)` | ||
| 4. Deploy, then create second migration with `SafeRemoveField(..., deletion_action=DeletionAction.DELETE)` | ||
| Deleting takes two migrations. Write both up front, but they must be **two separate PRs**, with phase 2 branched off phase 1 so its migration depends on it. Say clearly that **phase 2 can't merge until phase 1 has deployed** — merging them together drops the column while old code is still running. |
Contributor
There was a problem hiding this comment.
nit; stacking also implies the second can't merge before first so perhaps worth reducing the instruction further.
Suggested change
| Deleting takes two migrations. Write both up front, but they must be **two separate PRs**, with phase 2 branched off phase 1 so its migration depends on it. Say clearly that **phase 2 can't merge until phase 1 has deployed** — merging them together drops the column while old code is still running. | |
| Deleting takes two migrations. Write both up front, but they must be **two separate PRs**, with phase 2 stacked on top off phase 1 so its migration depends on it. Say clearly that **phase 2 can't merge until phase 1 has deployed** — merging them together drops the column while old code is still running. |
| Run `makemigrations` twice, in this order. Once the field is off the model Django can't generate the `AlterField` anymore, so doing it the other way around means silently shipping without it. | ||
|
|
||
| 1. With the field **still on the model**, edit it in place: `db_constraint=False` if it's an FK, `null=True` if it's not nullable and has no `db_default`. Run `makemigrations` to get the `AlterField`. | ||
| 2. Remove the field and every code reference to it, then `makemigrations` again. Replace the generated `RemoveField` with `SafeRemoveField(..., deletion_action=DeletionAction.MOVE_TO_PENDING)` — this drops the Django state, not the column. |
Contributor
There was a problem hiding this comment.
I know this is preexisting instruction, just a side note: wouldn't it be better if this was done deterministically in the script/makefile?
|
|
||
| 1. With the field **still on the model**, edit it in place: `db_constraint=False` if it's an FK, `null=True` if it's not nullable and has no `db_default`. Run `makemigrations` to get the `AlterField`. | ||
| 2. Remove the field and every code reference to it, then `makemigrations` again. Replace the generated `RemoveField` with `SafeRemoveField(..., deletion_action=DeletionAction.MOVE_TO_PENDING)` — this drops the Django state, not the column. | ||
| 3. Hand-merge both into one migration: |
Contributor
There was a problem hiding this comment.
Suggested change
| 3. Hand-merge both into one migration: | |
| 3. Hand-merge both into one migration. Example: |
| ### Removing a Model (and eventually its table) | ||
|
|
||
| Two-phase process — the `historical_silo_assignments` entry must be added in phase 1. | ||
| Dropping a table takes two migrations. Write both up front, but they must be **two separate PRs**, with phase 2 branched off phase 1 so its migration depends on it. Say clearly that **phase 2 can't merge until phase 1 has deployed** — merging them together drops the table while old code is still running. |
Contributor
There was a problem hiding this comment.
Suggested change
| Dropping a table takes two migrations. Write both up front, but they must be **two separate PRs**, with phase 2 branched off phase 1 so its migration depends on it. Say clearly that **phase 2 can't merge until phase 1 has deployed** — merging them together drops the table while old code is still running. | |
| Dropping a table takes two migrations. Write both up front, but they must be **two separate PRs**, with phase 2 stacked on top off phase 1 so its migration depends on it. Say clearly that **phase 2 can't merge until phase 1 has deployed** — merging them together drops the table while old code is still running. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Be more specific about exactly how to remove columns and tables