Skip to content

chore(migrations): Improve migration guidance - #121907

Open
wedamija wants to merge 1 commit into
masterfrom
danf/migration-deletion-guidance
Open

chore(migrations): Improve migration guidance#121907
wedamija wants to merge 1 commit into
masterfrom
danf/migration-deletion-guidance

Conversation

@wedamija

Copy link
Copy Markdown
Member

Be more specific about exactly how to remove columns and tables

Be more specific about exactly how to remove columns and tables
@wedamija
wedamija requested a review from a team as a code owner August 12, 2026 23:37

@vaind vaind left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some nits, I'd wait for owners-migrations approval either way

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants