Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#22564 closed Bug (fixed)

Migrations generated by Python 2 do not work in Python 3 due to b'' prefix on strings

Reported by: timo Owned by: claudep
Component: Migrations Version: 1.7-beta-2
Severity: Release blocker Keywords:
Cc: loic@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I think this is an issue if we expect apps to be able to support both Python 2 and 3.

  File "/home/tim/code/mysite/polls/migrations/0001_initial.py", line 7, in <module>
    class Migration(migrations.Migration):
  File "/home/tim/code/mysite/polls/migrations/0001_initial.py", line 28, in Migration
    (b'poll', models.ForeignKey(to=b'polls.Poll', to_field='id')),
  File "/home/tim/code/django/django/db/models/fields/related.py", line 1589, in __init__
    assert isinstance(to, six.string_types), "%s(%r) is invalid. First parameter to ForeignKey must be either a model, a model name, or the string %r" % (self.__class__.__name__, to, RECURSIVE_RELATIONSHIP_CONSTANT)
AssertionError: ForeignKey(b'polls.Poll') is invalid. First parameter to ForeignKey must be either a model, a model name, or the string 'self'

six.string_types is str on Python 3, not bytes.

Change History (11)

comment:1 Changed 12 months ago by claudep

+1, I also encountered this. We don't need prefixes, we now import unicode_literals in migration files.

comment:2 follow-up: Changed 12 months ago by loic84

  • Cc loic@… added

It's not easy, we do include unicode_literals in migrations so that PY2 and PY3 agree to speak the same dialect, but what's received by the serializer as a byte needs to remain a byte, hence the prefix.

We need to either relax the assertion, or ensure ForeignKey.deconstruct() returns a unicode string, preferably the latter. If I remember well, a bunch of meta are stored as byte, so I'm guessing that's how it lands in the output of ForeignKey.deconstruct().

comment:3 in reply to: ↑ 2 Changed 12 months ago by claudep

Replying to loic84:

...what's received by the serializer as a byte needs to remain a byte, hence the prefix.

Could you elaborate a bit about that assertion? What problem could arise if we always output "normal" strings.

comment:4 Changed 12 months ago by loic84

The serializer doesn't know much about the context, it receives objects one by one recursively, and when it finds a byte, it could come from anywhere, so it should IMO render it as a byte.

I don't think we should cast byte into unicode strings at serialization time, maybe the reconstructed object actually expected a byte, the proper fix is to feed the serializer with the expected type, either by normalizing the output of ForeignKey.deconstruct() to unicode, or by fixing it even deeper (probably in Model._meta).

comment:5 Changed 12 months ago by claudep

Fair, let's try to solve it above the serialization level.

comment:6 Changed 12 months ago by claudep

  • Owner changed from nobody to claudep
  • Status changed from new to assigned

comment:8 Changed 12 months ago by loic84

  • Triage Stage changed from Accepted to Ready for checkin

LGTM.

comment:9 Changed 12 months ago by Claude Paroz <claude@…>

In e8f1395f4e65f9d2ffc7a0448980ebaca0b2991a:

[1.7.x] Added a bunch of missing unicode_literals

Refs #22564.
Backport of 12474dace from master.

comment:10 Changed 12 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 0d138b9cf450b213b950595527b63d253926609d:

[1.7.x] Fixed #22564 -- Prevented unneeded bytestrings in migrations

In some cases, this could lead to migrations written with Python 2
being incompatible with Python 3.
Thanks Tim Graham for the report and Loïc Bistuer for the advices.
Backport of da9cf53cb from master.

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