#21498 closed Bug (invalid)
Changing locale causes migrations autodetector to make unnecesary changes
Reported by: | foonicorn | Owned by: | Andrew Godwin |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | info@…, loic@…, andrew@…, tomas.kostrhun@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
A migration is created after fields like help_text
changed, although this is not relevant for the database. This also occurs when i18n is used and the translation changes.
Change History (36)
comment:1 by , 11 years ago
Cc: | added |
---|---|
Owner: | set to |
Status: | new → assigned |
follow-up: 3 comment:2 by , 11 years ago
comment:3 by , 11 years ago
I'm going to implement it in a way that fields can define their own non-db-relevant arguments.
comment:4 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:5 by , 11 years ago
Cc: | added |
---|
comment:6 by , 11 years ago
I'd rather we included every single field - white/blacklisting fields is dangerous. What if a field subclass redefines help_text? What if it uses it to help populate ORM objects? I'd like to mark this WONTFIX.
comment:7 by , 11 years ago
+1, I'm particularly concerned by the fate of validators
which were blacklisted in the tentative patch presented on IRC.
Also with the upcoming VirtualField
we will be blurring the lines more and more between the ORM and the underlying schema.
However there seems to be an valid issue here, i18n locale shouldn't trip the autodetector, I haven't had time to look into it but I suspect this is due to the fix for #21008.
comment:8 by , 11 years ago
I think we should find a solution for this: imaging your translators change the translation for a help text. All the time they do one developer will end up with such a new migration.
The way I implemented it allows fields to redefine their attributes. It's a prove of concept: https://github.com/Markush2010/django/tree/ticket21498
comment:9 by , 11 years ago
I agree that the locale shouldn't trip the autodetector, but changing help_text
unfortunately has to (we just can't guarantee it's not useful at database or field runtime).
Perhaps we should force a certain locale while running the autodetector?
comment:10 by , 11 years ago
Cc: | added |
---|
comment:11 by , 11 years ago
Summary: | Unnecessary migrations after nonrelevat fields changed → Changing locale causes migrations autodetector to make unnecesary changes |
---|
comment:12 by , 11 years ago
To solve this we may need a better way to handle Promises, maybe we could make them "deconstructible"?
comment:13 by , 11 years ago
Well, the serializer currently runs any Promise through force_text before saving it, but the autodetector doesn't serialize anything before it compares. To be honest, if we could make Promises comparable (and make them evaluate on eq) that would solve it.
comment:14 by , 11 years ago
Owner: | changed from | to
---|---|
Severity: | Normal → Release blocker |
Claiming this to fix it tonight, as it just got way worse now we have migrations for contrib apps (upping to release blocker due to that)
comment:15 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:16 by , 11 years ago
Oddly, no fix was needed on the stable/1.7.x branch, as it did not show the symptom in my tests; this appears to have been fixed by a the new autodetector code and then broken by https://github.com/django/django/commit/a5f6cbce07b5f3ab48d931e3fd1883c757fb9b45.
If someone can get the problem on the stable/1.7.x branch please reopen.
comment:17 by , 10 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
comment:18 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
@ChillarAnand, please don't re-open without providing a valid reason.
Did you manage to reproduce the issue on the stable/1.7.x
branch?
comment:19 by , 10 years ago
I can reproduce it on 1.7.2.
Started the project with
LANGUAGE_CODE = 'cs' LANGUAGES = ( ('cs', 'Czech'), ('en', 'English'), ) USE_I18N = True
all model fields with verbose_name in Czech like
from django.db import models from django.utils.translation import ugettext_lazy as _ class Book(models.Model): name = models.CharField(_(u'Název'), max_length=255)
initial migration
('name', models.CharField(max_length=255, verbose_name='N\xe1zev')),
after I've added en locale and translated "Název" as "Title" makemigrations shows
migrations.AlterField( model_name='book', name='name', field=models.CharField(max_length=255, verbose_name='Title'), preserve_default=True, ),
when I import _ in models.py just as ugettext (not ugettext_lazy) problem disappears.
comment:20 by , 10 years ago
Cc: | added |
---|
follow-ups: 22 24 comment:21 by , 10 years ago
Hi Tomas, can you test with Django's master branch? I believe that issue might be related to/fixed by f7c287fca9c9e6370cc88d1457d3ed9466703687.
comment:22 by , 10 years ago
Replying to timgraham:
Hi Tomas, can you test with Django's master branch? I believe that issue might be related to/fixed by f7c287fca9c9e6370cc88d1457d3ed9466703687.
It's exactly the case I constructed in https://github.com/django/django/pull/3838#issuecomment-68784330
comment:23 by , 10 years ago
In Django 1.8, you *might* obtain a new migration just after having upgraded from 1.7 (if the existing migration uses a translated string), but then the translations won't affect migrations any longer.
comment:24 by , 10 years ago
Replying to timgraham:
Hi Tomas, can you test with Django's master branch? I believe that issue might be related to/fixed by f7c287fca9c9e6370cc88d1457d3ed9466703687.
Hello Tim, yes, django trunk shows "No changes detected" so it's OK.
comment:25 by , 9 years ago
This issue re-occurs on 1.9.2, I managed to overcome it by overwriting the makemigrations command then forcing a default locale.. perhaps this should be a django setting anyway, optionally adding the flag through cli?
comment:26 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
follow-up: 28 comment:27 by , 9 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
I was not able to reproduce it with Django 1.9.2. You might have to create a sample project where this can be reproduced to convince us.
comment:28 by , 8 years ago
Replying to Claude Paroz:
I was not able to reproduce it with Django 1.9.2. You might have to create a sample project where this can be reproduced to convince us.
I have created a sample project available at https://github.com/mbaechtold/django-ticket-21498-demo
Hope this helps to convince you this is still an issue.
follow-up: 32 comment:29 by , 8 years ago
Martin, many thanks for the sample project. Could you please try to reproduce the issue by replacing ugettext
by ugettext_lazy
in the polls/models.py
file?
comment:30 by , 8 years ago
Same problem here. makemigrations
executed from production server and local server would always generate new migration files. Not good for Git control.
App: django-avatar
diff ./avatar/migrations/0006_auto_20170329_0405.py ./avatar/migrations/0007_auto_20170329_0455.py 2c2 < # Generated by Django 1.10.5 on 2017-03-28 20:05 --- > # Generated by Django 1.10.5 on 2017-03-28 20:55 14c14 < ('avatar', '0005_auto_20170306_1939'), --- > ('avatar', '0006_auto_20170329_0405'), 20c20 < options={'ordering': ['-pk'], 'verbose_name': 'avatar', 'verbose_name_plural': 'avatars'}, --- > options={'ordering': ['-pk'], 'verbose_name': '头像', 'verbose_name_plural': '头像'}, 25c25 ...
comment:31 by , 8 years ago
It's a bug in django-avatar -- it uses from django.utils.translation import ugettext as _
rather than ugettext_lazy
as Claude mentioned.
comment:32 by , 8 years ago
Replying to Claude Paroz:
Martin, many thanks for the sample project. Could you please try to reproduce the issue by replacing
ugettext
byugettext_lazy
in thepolls/models.py
file?
Claude, sorry for the delay, I did not get a notification about your comment.
Replacing ugettext
by ugettext_lazy
in the polls/models.py
file did not solve the problem. New migration files are generated anyway when changing settings.LANGUAGE_CODE
.
comment:33 by , 8 years ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
comment:35 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Claude, I was somewhat rash with my previous statement. I created the demo project from scratch (locally only, not pushed to GitHub) using ugettext_lazy
and I could not reproduce the error. I assume this ticket can be closed. Thanks for your help!
comment:36 by , 8 years ago
Resolution: | fixed → invalid |
---|---|
Severity: | Release blocker → Normal |
FWIW I don't think it's a good idea to start blacklisting / whitelisting parameters, especially with custom fields in mind.