Skip to content

Fixed #32350 -- Fixed showmigrations crash for applied squashed migrations.#13890

Merged
felixxm merged 1 commit intodjango:masterfrom
danielebra:master
Feb 4, 2021
Merged

Fixed #32350 -- Fixed showmigrations crash for applied squashed migrations.#13890
felixxm merged 1 commit intodjango:masterfrom
danielebra:master

Conversation

@danielebra
Copy link
Copy Markdown
Contributor

@danielebra danielebra commented Jan 14, 2021

https://code.djangoproject.com/ticket/32350#ticket

Some migrations don't have an applied timestamp, this pull requests ensures a check takes place before attempting to show it.

@danielebra danielebra requested a review from charettes January 20, 2021 03:30
@felixxm felixxm self-assigned this Jan 20, 2021
@felixxm felixxm removed their assignment Jan 25, 2021
@danielebra danielebra requested a review from charettes January 28, 2021 01:05
Copy link
Copy Markdown
Member

@charettes charettes left a comment

Choose a reason for hiding this comment

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

Thanks for the adjustments @danielebra!

Per the documentation if replaces is used then the replaced migrations should still be around

You must then transition the squashed migration to a normal migration by:

  • Deleting all the migration files it replaces.
  • Updating all migrations that depend on the deleted migrations to depend on the squashed migration instead.
  • Removing the replaces attribute in the Migration class of the squashed migration (this is how Django tells that it is a squashed migration).

So I'd add test_migrations_initial_squash/0001_initial.py and test_migrations_initial_squash/0002_replaced.py as well. I managed to reproduce even with these files around but at least we're sure not to test an unsupported setup.

I'll submit a PR to deal with the Running (pre/post)-migrate handlers for application migrations messages during test runs.

@danielebra
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @charettes, apologies for the late reply.

I have added in the first and second migration steps. Please review when convenient.

@danielebra danielebra requested a review from charettes February 2, 2021 05:12
Copy link
Copy Markdown
Member

@charettes charettes left a comment

Choose a reason for hiding this comment

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

Beyond the stylistic adjustments this LGTM. Thanks for the patch @danielebra!

@felixxm felixxm self-assigned this Feb 4, 2021
@felixxm felixxm changed the title Ticket #32350 Showmigrations management command run time error Fixed #32350 -- Fixed showmigrations crash for applied squashed migrations. Feb 4, 2021
Copy link
Copy Markdown
Member

@felixxm felixxm left a comment

Choose a reason for hiding this comment

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

@danielebra Thanks for this patch 👍 Welcome aboard ⛵

I pushed small edits to tests and reused an existing set of migrations test_migrations_squashed.

@felixxm felixxm merged commit 3f8979e into django:master Feb 4, 2021
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.

3 participants