-
Notifications
You must be signed in to change notification settings - Fork 677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix migrate to bigautofield #411
Conversation
Thanks @mom1 two questions:
Thanks for this! |
For example if this pr will be include to 4.8.0
Thanks for this library! |
tl;dr will an additional migrations file that is the exact same as user generated migrations break Django because they are the exact same? Thanks for the diagnosis @mom1 ! I think what this PR has is good, but we need to make sure the release doesn't break any production builds, too. What we could do is create a new migrations file that is titled with a datetime (e.g. The remaining question is: we know for a fact that the user generated migrations file and the new file we're adding is the same, but will that break Django because they are the exact same? My intuition is saying "no," but obviously we'd need to test this. Thoughts? Sorry edit note:
Not everyone uses a virtual env for production or has multiple servers that are brought down and up all the time like on cloud hosted instances, so that's why I'm recommending this approach be added. |
@Andrew-Chen-Wang I understand your concerns. If the user created the file dependencies = [
('token_blacklist', '0008_migrate_to_bigautofield'),
] and the next time the migration is created, django will offer to create a third file with dependencies = [
('token_blacklist', '0009_auto_20210525_0101'),
('token_blacklist', '0009_auto_20210526_0101asdfh2jkfdu892'),
] |
@mom1 I'm assuming you're talking about the merge conflict and thus a new file that tries to linearize the migration history, right? Something like We create a new migration file 0010_auto_*.py based on the deleted import os, fnmatch
from pathlib import Path
from django.db import migrations
parent_dir = Path(__file__).resolve(strict=True).parent
class Migration(migrations.Migration):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.dependencies = [
('token_blacklist', '0010_auto_20210527_1746')
]
_m = sorted(fnmatch.filter(os.listdir(parent_dir), "000*.py"))
if len(_m) == 9:
self.dependencies.insert(0, ('token_blacklist', os.path.splitext(_m[8])[0]))
operations = [
] It looks quite unstable since we're using files to check, but I couldn't find a way to hack with SQL to the django_migrations table to grab which migrations had already been migrated. However, because of what was user-generated by 0009 migrations, we also have to do a bit of work in 0010: # Generated by Django 3.2.3 on 2021-05-27 17:46
import os, fnmatch
from pathlib import Path
from django.db import migrations, models
parent_dir = Path(__file__).resolve(strict=True).parent
class Migration(migrations.Migration):
dependencies = [
('token_blacklist', '0008_migrate_to_bigautofield'),
]
operations = [
migrations.AlterField(
model_name='blacklistedtoken',
name='id',
field=models.BigAutoField(primary_key=True, serialize=False),
),
migrations.AlterField(
model_name='outstandingtoken',
name='id',
field=models.BigAutoField(primary_key=True, serialize=False),
),
] Finally, we add those id = models.BigAutoField(primary_key=True, serialize=False)
# This goes into both OutstandingToken and BlacklistedToken So those three changes (two new files, and one changed file i.e. models.py) makes sure no new migrations file is needed. Sorry for not trying this out earlier; was just swamped by work. Let me know if you find anything errneous! Note: I wrote all this code down here since I'd like for you to create a PR for it. I can't approve my own PRs, so your help is really appreciated! |
Sounds great! p.s. Added your code. |
@mom1 Unfortunately I tried it without adding and those migrating from 4.6.0 to 4.8.0 or who didn't migrate would need to create a new migrations file like those who did migrate on 4.7.0, so it's necessary. |
Fix #410