Opened 2 years ago

Last modified 2 years ago

#24964 assigned Bug

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 (12)

comment:1 Changed 2 years ago by Dave Arter

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

comment:2 Changed 2 years ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:3 Changed 2 years ago by Andriy Sokolovskiy

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 Changed 2 years ago by Andriy Sokolovskiy

Cc: me@… added

comment:5 Changed 2 years ago by Andriy Sokolovskiy

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 Changed 2 years ago by Andriy Sokolovskiy

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?

comment:7 in reply to:  5 ; Changed 2 years ago by Marten Kenbeek

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 in reply to:  7 Changed 2 years ago by Andriy Sokolovskiy

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 Changed 2 years ago by Claude Paroz

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

comment:10 in reply to:  9 Changed 2 years ago by Carl Meyer

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 Changed 2 years ago by Andriy Sokolovskiy

Ticket seems to be stale.
Do we need to change behavior of resolving lazy empty string, or change behavior in deconstruction?

comment:12 Changed 2 years ago by Tim Graham

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.

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