Opened 9 years ago

Closed 4 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 of ugettext_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 Dave Arter, 9 years ago

A quick git bisect indicates this was introduced in 11f307a5a8fa66605652e0496c7385e584bfcad7.

comment:2 by Tim Graham, 9 years ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:3 by Andriy Sokolovskiy, 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 Andriy Sokolovskiy, 9 years ago

Cc: me@… added

comment:5 by Andriy Sokolovskiy, 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 Andriy Sokolovskiy, 9 years ago

Owner: changed from nobody to Andriy Sokolovskiy
Status: newassigned

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?

in reply to:  5 ; comment:7 by Marten Kenbeek, 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''.

in reply to:  7 comment:8 by Andriy Sokolovskiy, 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 from object, which is what you're seeing. If you use str(new_field.help_text), you should see u''.

I described problem in https://code.djangoproject.com/ticket/24964#comment:6, this is not related to actual problem

comment:9 by Claude Paroz, 9 years ago

Do we have at least a use case for help_text=_('')?

in reply to:  9 comment:10 by Carl Meyer, 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 Andriy Sokolovskiy, 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 Tim Graham, 9 years ago

Severity: Release blockerNormal

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 Claude Paroz, 4 years ago

Resolution: wontfix
Status: assignedclosed

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.

Note: See TracTickets for help on using tickets.
Back to Top