Opened 9 years ago

Closed 9 years ago

#26429 closed Bug (fixed)

Name clash of merge migrations after squashing

Reported by: xgenadam Owned by: Raphael Gaschignard
Component: Migrations Version: 1.9
Severity: Normal Keywords: makemigrations merge clash
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Daniel Izquierdo)


Change History (12)

comment:1 by Tim Graham, 9 years ago

For clarity, could you give the specific steps that led to the clash?

comment:2 by Tim Graham, 9 years ago

Resolution: needsinfo
Status: newclosed

comment:3 by Raphael Gaschignard, 9 years ago

I just ran into a similar problem, going to detail what happened:

  • Over the course of development, do a merge migration. In our case, it was the 21st migration so got named 0021_merge.py
  • Squash our migrations, deleting 0021_merge.py
  • Continue development, and through sheer luck, our next 2st migration is also a merge commit. It gets named 0021_merge.py
  • When running migrations, because the original migration was run, this new 0021_merge.py commit is not run

In my case, this was the dependencies of 0021_merge.py (the new one):

    dependencies = [
        ('document', '0020_auto_20160413_1834'),
        ('document', '0019_auto_20160414_1150'),
    ]

Because the merge commit wasn't run, some people only had the 0020 mainline migrations migrated, some people only had the 0019 branch migrations run, and some people (who hadn't run migrate in a while) had the following happen:

  • migrate detects 0021_merge.py as the leaf node
  • there was record of the (old) 0021_merge.py migration
  • the system was like "oh, guess you're up to date!" and then doesn't run either branch of the merge commit

Like the OP, I believe the simplest solution is to timestamp the merge migrations.

comment:4 by Raphael Gaschignard, 9 years ago

Resolution: needsinfo
Status: closednew

comment:5 by Raphael Gaschignard, 9 years ago

Did a bit more digging into this, I believe that the fix is changing [(https://github.com/django/django/blob/master/django/core/management/commands/makemigrations.py#L286)], from :

new_migration = subclass("%04i_merge" % (biggest_number + 1), app_label) 

to :

timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M")
new_migration = subclass("%04i_merge_%s" % (biggest_number + 1, timestamp), app_label) 

I'm not quite sure how to write tests for this, though. You could write some models, generate migrations, squash, generate new ones and then confirm that everything got run? But I feel like the current squash migration tests also cover this, and that there isn't really much in term of name conflict tests.

comment:6 by Daniel Izquierdo, 9 years ago

Description: modified (diff)

The docs in https://github.com/django/django/blob/2cd2d188516475ddf256e6267cd82c495fb5c430/django/db/migrations/autodetector.py#L1129 say that names are not guaranteed to be unique and the timestamp is just a best effort to avoid conflicts. So I think rtpg's suggested fix works well for merge migrations, and a test to ensure names don't conflict is not necessary.

comment:7 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted

comment:8 by Raphael Gaschignard, 9 years ago

Owner: changed from nobody to Raphael Gaschignard
Status: newassigned

comment:9 by Tim Graham, 9 years ago

Has patch: set

comment:10 by Tim Graham, 9 years ago

Patch needs improvement: set

Tests need to be fixed on the PR.

comment:11 by Tim Graham, 9 years ago

Patch needs improvement: unset

comment:12 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In 8f6a1a15:

Fixed #26429 -- Added a timestamp to merge migration names.

This reduces the possibility of a naming conflict, especially after
squashing migrations.

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