Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#22474 closed Bug (fixed)

Bad migration detection in multiple database setup

Reported by: Claude Paroz Owned by: nobody
Component: Migrations Version: 1.7-beta-1
Severity: Release blocker Keywords:
Cc: 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

While trying to play with migrations in contrib apps, I stumble upon test issues, and finally found out that the migration was only applied to the first database ('default'), because while setting the second test database ('other'), MigrationRecorder was querying the first (default) database to detect if the migration was applied or not.

Attachments (2)

22474-1.diff (2.2 KB) - added by Claude Paroz 2 years ago.
Use the proper database to query for Migrations
22474-2.diff (3.2 KB) - added by Claude Paroz 2 years ago.
Include classmethod fix and basic test

Download all attachments as: .zip

Change History (9)

Changed 2 years ago by Claude Paroz

Attachment: 22474-1.diff added

Use the proper database to query for Migrations

comment:1 Changed 2 years ago by Claude Paroz

Needs tests: set

comment:2 Changed 2 years ago by Claude Paroz

Patch needs improvement: set

And just realized flush is a classmethod without access to self.connection :-/

Changed 2 years ago by Claude Paroz

Attachment: 22474-2.diff added

Include classmethod fix and basic test

comment:3 Changed 2 years ago by Claude Paroz

Needs tests: unset
Patch needs improvement: unset

comment:4 Changed 2 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:5 Changed 2 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

Might be worth factoring self.Migration.objects.using(self.connection.alias) into a property on the class? Otherwise, looks good.

comment:6 Changed 2 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In 7c54f8cced37313572d72f59bd2ef7ce12aa8fe1:

Fixed #22474 -- Made migration recorder aware of multiple databases

Thanks Tim Graham for the review.

comment:7 Changed 2 years ago by Claude Paroz <claude@…>

In 1084456ac2e707fc562906e95ad78f409dcc9325:

[1.7.x] Fixed #22474 -- Made migration recorder aware of multiple databases

Thanks Tim Graham for the review.
Backport of 7c54f8cce from master.

Note: See TracTickets for help on using tickets.
Back to Top