Opened 5 years ago

Closed 4 years ago

#24949 closed Bug (wontfix)

Force to_field and probably other fields to unicode during migration deconstruction

Reported by: Markus Holtermann Owned by: nobody
Component: Migrations Version: 1.7
Severity: Normal Keywords: py2
Cc: Markus Holtermann Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

to_field and likely many other field attributes are not forced to unicode during migration creation. This is the same problem as reported in #23455.

We should figure out a way how to fix this issue once and for all.

Change History (7)

comment:1 Changed 5 years ago by Markus Holtermann

Triage Stage: UnreviewedAccepted

After a brief talk with Andrew we agreed on converting all string attributes to unicode (force_text()) in the fields' __init__() methods. That should fix the issue and possibly other issues in the future. There are a few things to consider, though:

  • For BinaryField we need to keep the byte string, though for CharField and TextField it should be unicode.
  • The verbose_name and help_text attributes are often used with (u)gettext_lazy(). Although I'm lacking a real use case for gettext_lazy() in general, we need to find a way to force these values to unicode once they are evaluated.

comment:2 Changed 5 years ago by Tim Graham

Did we decide the documented solution doesn't work or merely that it's inconvenient?

comment:3 Changed 5 years ago by Claude Paroz

As for me, I'd be in favor of Markus proposal.
@tim, of course, the documented solution works, but in practice, many people are bitten by this (as they obviously don't begin by reading the docs) and I often find myself manually removing b prefixes from migration files.

comment:4 Changed 5 years ago by Tim Graham

#25906 notes that this can also affect AppConfig.app_name when squashing migrations.

comment:5 Changed 5 years ago by Patryk Zawadzki

It's also the case if your app does not have an AppConfig file and your INSTALLED_APPS is a list of byte strings.

As I've noted in #25906, I'm not sure working around problems like this at Django level is worth it. It will add a lot of seemingly no-op code that in edge cases could silently cover entire classes of unrelated problems.

I think it would be a sane thing for Django to warn about things being the wrong type and do nothing about it. Affected apps and projects will continue to work in Python 2 and fail in Python 3 but they do that already (they may not know or care). A warning should give enough information to fix the problem (or file a bug with app's upstream).

I was affected and it took me a couple of months to find out. Our entire test suite passed with flying colours and things only broke when squashing migrations. With a proper validator warning it becomes a five minute fix.

comment:6 Changed 4 years ago by Tim Graham

Keywords: py2 added

If someone doesn't propose a patch for Django 1.11 (last version to support Python 2) we can likely close the ticket.

comment:7 Changed 4 years ago by Tim Graham

Resolution: wontfix
Status: newclosed

Closing due to the end of Python 2 support in master in a couple weeks.

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