Opened 9 years ago
Closed 5 years ago
#24964 closed Bug (wontfix)
Infinite migrations with empty help_text via ugettext_lazy
Reported by: | Andy Gimblett | Owned by: | Andriy Sokolovskiy |
---|---|---|---|
Component: | Migrations | Version: | 1.8 |
Severity: | Normal | Keywords: | migrations, internationalisation |
Cc: | me@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I've come across something which seems to be a bug; I've asked about it on django-users and was encouraged to raise it here.
There's a fuller explanation and easily runnable example on github: https://github.com/gimbo/django-ugettext_lazy_migrations
In short, given the following model (note the empty help_text
string),
from django.db import models from django.utils.translation import ugettext_lazy as _ class X(models.Model): y = models.CharField(help_text=_(''), max_length=10)
repeated calls of manage.py makemigrations
will, after the initial migration, produce an infinite series of new migrations, each of which contains an AlterField
, but which does nothing. The expected behaviour would be to produce an initial migration and then no further migrations, of course.
The problems goes away if you do any of the following:
- Remove the
help_text
parameter - Use a non-empty
help_text
- Don't internationalise the
help_text
value - Use
ugettext
instead ofugettext_lazy
(which, I understand, you shouldn't)
I've reproduced it on all released 1.8.x Django versions up to 1.8.2, but it doesn't exhibit on any 1.7.x versions I've tried.
I haven't tried any development versions.
I mentioned it on django-users, and Carl Meyer suggested the following:
I would guess the problem has to do with a careless
if help_text:
somewhere in the migration detection code. It would be worth checking whether the same problem exists with any other similar field parameters (e.g.label
).
Thanks!
Change History (13)
comment:1 by , 9 years ago
comment:2 by , 9 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 9 years ago
A quick look to the issue
ipdb> old_field_dec (u'django.db.models.PositiveSmallIntegerField', [], {u'verbose_name': u'action flag'}) ipdb> new_field_dec (u'django.db.models.PositiveSmallIntegerField', [], {u'verbose_name': <django.utils.functional.__proxy__ object at 0x1085c1250>})
I'm not sure the behavior in https://github.com/django/django/commit/11f307a5a8fa66605652e0496c7385e584bfcad7 need to be changed, so maybe autodetector changes are needed.
comment:4 by , 9 years ago
Cc: | added |
---|
follow-up: 7 comment:5 by , 9 years ago
Or, the question is more why
ipdb> old_field.help_text u'' ipdb> new_field.help_text <django.utils.functional.__proxy__ object at 0x10d78bf50>
comment:6 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
When autodetector is doing deconstruction, there is https://github.com/django/django/blob/3c593ba79e5bdee46419e2aa6b14e7ce8bc01758/django/db/models/fields/__init__.py#L408
For example, in our case initial migration was with help_text=_('')
resolved to ''
, simple empty string.
Later, there is https://github.com/django/django/blob/3c593ba79e5bdee46419e2aa6b14e7ce8bc01758/django/db/models/fields/__init__.py#L432. So, this value is ignored, and not added to deconstruction, so makemigration will produce it as a change.
If I will add migration with help_text=''
(without lazy), it will appear in migration as b''
, so the check above will work and it will appear in deconstruction.
So, do we need to change behavior of resolving lazy empty string, or change behavior in deconstruction?
follow-up: 8 comment:7 by , 9 years ago
Replying to coldmind:
Or, the question is more why
ipdb> old_field.help_text u'' ipdb> new_field.help_text <django.utils.functional.__proxy__ object at 0x10d78bf50>
__repr__
is not in any way set on __proxy__
, so it inherits the default behaviour from object
, which is what you're seeing. If you use str(new_field.help_text)
, you should see u''
.
comment:8 by , 9 years ago
Replying to knbk:
Replying to coldmind:
Or, the question is more why
ipdb> old_field.help_text u'' ipdb> new_field.help_text <django.utils.functional.__proxy__ object at 0x10d78bf50>
__repr__
is not in any way set on__proxy__
, so it inherits the default behaviour fromobject
, which is what you're seeing. If you usestr(new_field.help_text)
, you should seeu''
.
I described problem in https://code.djangoproject.com/ticket/24964#comment:6, this is not related to actual problem
comment:10 by , 9 years ago
Replying to claudep:
Do we have at least a use case for
help_text=_('')
?
I don't, but this strikes me as a bug that deserves fixing even with no good use case for that. You should have to work harder than this to trigger infinite migrations...
comment:11 by , 9 years ago
Ticket seems to be stale.
Do we need to change behavior of resolving lazy empty string, or change behavior in deconstruction?
comment:12 by , 9 years ago
Severity: | Release blocker → Normal |
---|
Given the lack of use case for a translated empty string, I guess we can probably demote it from a release blocker.
comment:13 by , 5 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
I don't think the use case is strong enough to warrant working on a fix here. Of course, any patch would still be accepted.
A quick
git bisect
indicates this was introduced in 11f307a5a8fa66605652e0496c7385e584bfcad7.