Opened 4 years ago
Closed 4 years ago
#31769 closed Cleanup/optimization (fixed)
Improve default name of merge migrations.
Reported by: | Jon Dufresne | Owned by: | Jon Dufresne |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | 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
Currently, merge migrations filenames are created with a timestamp. For example:
0003_merge_20160102_0304.py
This name is more opaque than necessary. When one reads it, it isn't immediately clear which migrations were merged. One must inspect the file to find that information.
Instead, I suggest the default filename should be created by combining the files being merged. This way, it includes the information without requiring one to inspect the file. This information is also useful in the migrate
command's output. As it is most likely to merge two migrations files, this should remain a reasonable length.
For example:
0003_merge_0002_conflicting_second_0002_second.py
If preferable, we could decide to join the names in other ways too. For example, separate names by __
(two underscores) or exclude prefix numbers. To start, I'll go with the easiest approach unless there is strong preference for a different naming scheme.
Change History (7)
comment:1 by , 4 years ago
Has patch: | set |
---|
comment:2 by , 4 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Summary: | Improve default name of merge migrations → Improve default name of merge migrations. |
I'm not sure about this change because you may need to merge many migrations. In such case it seems that the timestamp is the best we can do, e.g. (with your patch):
$ python manage.py migrate CommandError: Conflicting migrations detected; multiple leaf nodes in the migration graph: (0002_mymodel1_field_1, 0002_mymodel10_mymodel2_mymodel3_mymodel4_mymodel5_mymodel6_mymodel7_mymodel8_mymodel9, 0002_mymodel101_mymodel21_mymodel31_mymodel41_mymodel51_mymodel61_mymodel71_mymodel81_mymodel91 in test_one). To fix them run 'python manage.py makemigrations --merge' $ python manage.py makemigrations --merge Merging test_one Branch 0002_mymodel10_mymodel2_mymodel3_mymodel4_mymodel5_mymodel6_mymodel7_mymodel8_mymodel9 - Create model MyModel1 - Create model ... Branch 0002_mymodel1_field_1 - Add field field_1 to mymodel1 Branch 0002_mymodel101_mymodel21_mymodel31_mymodel41_mymodel51_mymodel61_mymodel71_mymodel81_mymodel91 - Create model MyModel101 - Create model ... Merging will only work if the operations printed above do not conflict with each other (working on different fields or models) Do you want to merge these migration branches? [y/N] y Traceback (most recent call last): .... res = handle_func(*args, **kwargs) File "/django/django/core/management/commands/makemigrations.py", line 140, in handle return self.handle_merge(loader, conflicts) File "/django/django/core/management/commands/makemigrations.py", line 309, in handle_merge with open(writer.path, "w", encoding='utf-8') as fh: OSError: [Errno 36] File name too long: '/ticket_31769/test_one/migrations/0003_merge_0002_mymodel101_mymodel21_mymodel31_mymodel41_mymodel51_mymodel61_mymodel71_mymodel81_mymodel91_0002_mymodel10_mymodel2_mymodel3_mymodel4_mymodel5_mymodel6_mymodel7_mymodel8_mymodel9_0002_mymodel1_field_1.py'
Truncating merged names will be even more confusing.
You always use the --name
option if you don't like a generated name (maybe we could ask for it in the interactive
mode 🤔).
comment:3 by , 4 years ago
Resolution: | wontfix |
---|---|
Status: | closed → new |
I'm not sure about this change because you may need to merge many migrations. In such case it seems that the timestamp is the best we can do
In the case or merging lots of files, I agree the timestamp is the best we can do.
To solve this, we can add a threshold for the name length and if exceeded fallback to the old timestamp naming. This way, the common case of merging two or three migrations has a more useful name, but it doesn't get out of hand when more are involved. I'll update the PR with this change and include a test.
Having a threshold for auto-generated migration names has precedent, so we can carry that over:
The link uses a length of 100, which I'll use too. But I'm happy to tweak this larger or smaller to an agreeable size.
I'm also still open to dropping the migration number (0000_
) if that makes the filename more readable. I personally like including the number, but I'm open to alternatives.
You always use the --name option if you don't like a generated name
Yes. This is the status quo and my current approach. But it doesn't solve my use case of providing more useful name by default. This is especially important when working on a diverse team where some members will not always remember to use useful names. Catching this before the code gets to team review is helpful for all by reducing repetitive feedback and by reducing the work a computer can do.
Beyond that, improving the default now means more projects will likely have more useful and reproducible filenames moving forward, regardless of their internal standards.
There have recently been a few other PRs, tickets and discussions on improving migration names. IMO, this ticket is along those same goals:
https://groups.google.com/forum/#!msg/django-developers/Bmcd779Wdl4/mlQRVNVgAQAJ
https://github.com/django/django/commit/6f3e3e87ab72eb64dcce8e646f8eea07be6c3b7e
comment:4 by , 4 years ago
Owner: | changed from | to
---|---|
Patch needs improvement: | set |
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
I'm also still open to dropping the migration number (0000_) if that makes the filename more readable. I personally like including the number, but I'm open to alternatives.
IMO numbers should be included.
comment:5 by , 4 years ago
Patch needs improvement: | unset |
---|
comment:6 by , 4 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
https://github.com/django/django/pull/13160