[Django] #21498: Unnecessary migrations after nonrelevat fields changed
Django
----------------------------+--------------------
Reporter: foonicorn | Owner:
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* status: new => assigned
* cc: info@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* owner: => MarkusH
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:1>
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by loic84):
FWIW I don't think it's a good idea to start blacklisting / whitelisting
parameters, especially with custom fields in mind.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:2>
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
I'm going to implement it in a way that fields can define their own non-
db-relevant arguments.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:3>
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:4>
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by loic84):
* cc: loic@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:5>
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by andrewgodwin):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:6>
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by loic84):
+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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:7>
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:8>
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by andrewgodwin):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:9>
Django
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by andrewgodwin):
* cc: andrew@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:10>
Django
----------------------------+------------------------------------
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:11>
Django
----------------------------+------------------------------------
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by loic84):
To solve this we may need a better way to handle Promises, maybe we could
make them "deconstructible"?
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:12>
Django
----------------------------+------------------------------------
Reporter: foonicorn | Owner: MarkusH
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by andrewgodwin):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:13>
Django
Reporter: foonicorn | Owner: andrewgodwin
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by andrewgodwin):
* owner: MarkusH => andrewgodwin
* severity: Normal => Release blocker
Comment:
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)
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:14>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Andrew Godwin <andrew@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"d359647715bccffd7cfc5eb8bcf5edb5c714fb00"]:
{{{
#!CommitTicketReference repository=""
revision="d359647715bccffd7cfc5eb8bcf5edb5c714fb00"
Fixed #21498: Don't use a fallback language if you're en-us.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:15>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by andrewgodwin):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:16>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ChillarAnand):
* status: closed => new
* resolution: fixed =>
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:17>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* status: new => closed
* resolution: => fixed
Comment:
@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?
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:18>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tcx):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:19>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by tcx):
* cc: tomas.kostrhun@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:20>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Hi Tomas, can you test with Django's master branch? I believe that issue
might be related to/fixed by f7c287fca9c9e6370cc88d1457d3ed9466703687.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:21>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
Replying to [comment:21 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
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:22>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:23>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tcx):
Replying to [comment:21 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:24>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Hedde):
This issue re-occurs on 1.9.2, I managed to overcome it by overwriting the
makemigrations command then foricing a default locale.. perhaps this
should be a django setting anyway, optionally adding the falg through cli?
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:25>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
* status: closed => new
* resolution: fixed =>
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:26>
Django
---------------------------------+----------------------------------------
Reporter: foonicorn | Owner: andrewgodwin
Component: Migrations | Version: master
Severity: Release blocker | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
* status: new => closed
* resolution: => needsinfo
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:27>
Django
Reporter: foonicorn | Owner: Andrew Godwin
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Release blocker | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Martin Bächtold):
Replying to [comment:27 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 create a sample project available at https://github.com/mbaechtold
/django-ticket-21498-demo
Hope this helps to convince you this is still an issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:28>
Django
---------------------------------+-----------------------------------------
Reporter: foonicorn | Owner: Andrew Godwin
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Release blocker | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Comment (by Claude Paroz):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:29>
Django
---------------------------------+-----------------------------------------
Reporter: foonicorn | Owner: Andrew Godwin
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Release blocker | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Comment (by cryptogun):
Same problem here. `makemigrations` executed from production server and
local server would always generate new migration files. Not good for Git
control.
App: [https://github.com/grantmcconnaughey/django-
avatar/blob/master/avatar/models.py#L98 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
...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:30>
Django
---------------------------------+-----------------------------------------
Reporter: foonicorn | Owner: Andrew Godwin
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Release blocker | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Comment (by Tim Graham):
It's a bug in django-avatar -- it uses `from django.utils.translation
import ugettext as _` rather than `ugettext_lazy` as Claude mentioned.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:31>
Django
---------------------------------+-----------------------------------------
Reporter: foonicorn | Owner: Andrew Godwin
Type: Bug | Status: closed
Component: Migrations | Version: master
Severity: Release blocker | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Comment (by Martin Bächtold):
Replying to [comment:29 Claude Paroz]:
> 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?
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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:32>
Django
---------------------------------+-----------------------------------------
Reporter: foonicorn | Owner: Andrew Godwin
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
* status: closed => new
* resolution: needsinfo =>
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:33>
Django
---------------------------------+-----------------------------------------
Reporter: foonicorn | Owner: Andrew Godwin
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
Comment (by Claude Paroz):
Martin, could you then update your sample project to use `ugettext_lazy`?
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:34>
Django
---------------------------------+-----------------------------------------
Reporter: foonicorn | Owner: Andrew Godwin
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
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!
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:35>
Django
----------------------------+-----------------------------------------
Reporter: foonicorn | Owner: Andrew Godwin
Type: Bug | Status: closed
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* resolution: fixed => invalid
* severity: Release blocker => Normal
--
Ticket URL: <https://code.djangoproject.com/ticket/21498#comment:36>