[Django] #23455: migrations created with python2 break with python3
Django
----------------------------+--------------------
Reporter: brian | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
As an example, my model has:
{{{
@python_2_unicode_compatible
class Institute(models.Model):
...
delegates = models.ManyToManyField(
Person, related_name='delegate_for',
blank=True, null=True, through='InstituteDelegate')
...
}}}
makemigrations running under Python2 turns this into:
{{{
migrations.AddField(
model_name='institute',
name='delegates',
field=models.ManyToManyField(related_name=b'delegate_for', null=True,
through='karaage.InstituteDelegate', to='karaage.Person', blank=True),
preserve_default=True,
),
}}}
This works fine under Python2, where `b''` == `''`.
Under Python3 however, I get:
{{{
Traceback (most recent call last):
File "./manage.py", line 7, in <module>
management.execute_from_command_line()
File "/usr/lib/python3/dist-
packages/django/core/management/__init__.py", line 385, in
execute_from_command_line
utility.execute()
File "/usr/lib/python3/dist-
packages/django/core/management/__init__.py", line 377, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/usr/lib/python3/dist-
packages/django/core/management/commands/test.py", line 50, in
run_from_argv
super(Command, self).run_from_argv(argv)
File "/usr/lib/python3/dist-packages/django/core/management/base.py",
line 288, in run_from_argv
self.execute(*args, **options.__dict__)
File "/usr/lib/python3/dist-
packages/django/core/management/commands/test.py", line 71, in execute
super(Command, self).execute(*args, **options)
File "/usr/lib/python3/dist-packages/django/core/management/base.py",
line 338, in execute
output = self.handle(*args, **options)
File "/usr/lib/python3/dist-
packages/django/core/management/commands/test.py", line 88, in handle
failures = test_runner.run_tests(test_labels)
File "/usr/lib/python3/dist-packages/django/test/runner.py", line 147,
in run_tests
old_config = self.setup_databases()
File "/usr/lib/python3/dist-packages/django/test/runner.py", line 109,
in setup_databases
return setup_databases(self.verbosity, self.interactive, **kwargs)
File "/usr/lib/python3/dist-packages/django/test/runner.py", line 299,
in setup_databases
serialize=connection.settings_dict.get("TEST_SERIALIZE", True),
File "/usr/lib/python3/dist-packages/django/db/backends/creation.py",
line 374, in create_test_db
test_flush=True,
File "/usr/lib/python3/dist-
packages/django/core/management/__init__.py", line 115, in call_command
return klass.execute(*args, **defaults)
File "/usr/lib/python3/dist-packages/django/core/management/base.py",
line 338, in execute
output = self.handle(*args, **options)
File "/usr/lib/python3/dist-
packages/django/core/management/commands/migrate.py", line 160, in handle
executor.migrate(targets, plan, fake=options.get("fake", False))
File "/usr/lib/python3/dist-packages/django/db/migrations/executor.py",
line 63, in migrate
self.apply_migration(migration, fake=fake)
File "/usr/lib/python3/dist-packages/django/db/migrations/executor.py",
line 91, in apply_migration
if self.detect_soft_applied(migration):
File "/usr/lib/python3/dist-packages/django/db/migrations/executor.py",
line 135, in detect_soft_applied
apps = project_state.render()
File "/usr/lib/python3/dist-packages/django/db/migrations/state.py",
line 67, in render
model.render(self.apps)
File "/usr/lib/python3/dist-packages/django/db/migrations/state.py",
line 311, in render
body,
File "/usr/lib/python3/dist-packages/django/db/models/base.py", line
171, in __new__
new_class.add_to_class(obj_name, obj)
File "/usr/lib/python3/dist-packages/django/db/models/base.py", line
300, in add_to_class
value.contribute_to_class(cls, name)
File "/usr/lib/python3/dist-
packages/django/db/models/fields/related.py", line 2239, in
contribute_to_class
super(ManyToManyField, self).contribute_to_class(cls, name)
File "/usr/lib/python3/dist-
packages/django/db/models/fields/related.py", line 269, in
contribute_to_class
'app_label': cls._meta.app_label.lower()
TypeError: unsupported operand type(s) for %: 'bytes' and 'dict'
}}}
If I run makemigrations under Python 3, it does the correct thing:
{{{
migrations.AddField(
model_name='institute',
name='delegates',
field=models.ManyToManyField(related_name='delegate_for', null=True,
through='karaage.InstituteDelegate', to='karaage.Person', blank=True),
preserve_default=True,
),
}}}
If I look at a diff, it seems that just about everywhere a string (in the
above example, `through` and `to` appear to be ok) was used it is
incorrectly quoted in the migration as a byte string.
Apologies if this is already reported or documented somewhere, I looked
but couldn't see anything.
I have a vague recollection this may not have been a problem in the RC1
release, not absolutely sure now.
--
Ticket URL: <https://code.djangoproject.com/ticket/23455>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: brian | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.6
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by brian):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Urgh. Missed out on ticket 23456 by one :-(.
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:1>
Django
Reporter: brian | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* version: 1.6 => 1.7
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Managed to reproduce with the latest 1.7 release.
I guess a fix similar to 5257b85ab8a4a86b24005e3ca8c542ede44b0687 (#23226)
applied to field options should do here.
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:2>
Django
Reporter: brian | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I don't think it's a good idea to coerce field options. For example, you
might have `models.BinaryField(default=b"")`. How about instead
documenting this issue and recommending adding `unicode_literals` as
appropriate. For example, I did this for `contrib/contenttypes/models.py`
when adding migrations there in eb8600a65673649ea15ed18d17127f741807ac8b.
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:3>
Django
Reporter: brian | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markush2010):
* cc: info+coding@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:4>
Django
Reporter: brian | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by brian):
Replying to [comment:3 timgraham]:
It seems unfortunate that this means altering the previously working
source code, however I very much agree with your reasoning.
I would suggest that assertions be added to ensure that certain fields,
e.g. 'related_name' are always unicode strings (u"" for Python2/3 or ""
for Python3). i.e. ensure that broken code breaks for Python 2 as well as
Python 3, and possibly with a more consistent and friendly message.
Obviously this would break code that appears to be fine under Python 2.
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:5>
Django
Reporter: brian | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Sure, for most of the fields, the binary prefix makes no sense. I think
that the `unicode_literals` recommendation and the conversion of some
fields are no antagonistic solutions.
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:6>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: assigned
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markush2010):
* status: new => assigned
* owner: nobody => Markush2010
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:7>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: assigned
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markush2010):
* has_patch: 0 => 1
Comment:
I added a pull-request: https://github.com/django/django/pull/3276
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:8>
Django
Reporter: brian | Owner: Markush2010
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"45bd7b3bd9008941580c100b9fc7361e3ff3ff0d"]:
{{{
#!CommitTicketReference repository=""
revision="45bd7b3bd9008941580c100b9fc7361e3ff3ff0d"
Fixed #23455 -- Forced related_name to be a unicode string during
deconstruction.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:9>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e8a08514de6b97de1fce13b6c6b24cf72d1d60d9"]:
{{{
#!CommitTicketReference repository=""
revision="e8a08514de6b97de1fce13b6c6b24cf72d1d60d9"
[1.7.x] Fixed #23455 -- Forced related_name to be a unicode string during
deconstruction.
Backport of 45bd7b3bd9 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:10>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
I think this change should probably be reverted, and we should avoid any
auto-conversion that introduces a difference between real models and
migration models. See further discussion on #23982.
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:11>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
Thanks to feedback from charettes on
https://github.com/django/django/pull/3718, I've realized that this needs
to be fixed somehow, we can't only revert the existing fix - the error
message is too obscure.
But I think a better fix would be for Django to just always convert
`related_name` to text, not to do it specifically on deconstruction for
migrations. Rationale:
1) In general, the goal should be as much as possible to maintain uniform
behavior between Python 2 and Python 3.
2) With the current fix here, the situation is "on Python 2, related_name
can be bytes or text; on Python 3 it must be text." Migrations generated
on Python 2 and run on Python 3 expose this difference. So currently we
work around it by ensuring that Python 2 stores `related_name` as text in
migrations.
3) With my proposed fix, we maintain better consistency. Migrations always
preserves bytes/text, and Django always (regardless of Python version)
supports setting `related_name` as either bytes or text, and converts it
to text internally (since it needs that for interpolation).
The effect on generated migrations in the end will be the same as the
current fix; since `related_name` is converted to text, it will still be
deconstructed as text. The most noticeable difference is that with my
proposed fix, migrations generated on Django 1.7.0 under Python 2 won't
need to be manually edited to work under Python 3, they'll just work.
Is this reasonable, or am I missing something?
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:12>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
It seems reasonable to me.
I also think migrations should always preserve bytes/text and this
shouldn't be special cased at the migration level but handled at the model
one.
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:13>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carl Meyer <carl@…>):
In [changeset:"c72eb80d114fb5d90bd21b5549e8abd0bbd17f99"]:
{{{
#!CommitTicketReference repository=""
revision="c72eb80d114fb5d90bd21b5549e8abd0bbd17f99"
Fixed #23455 -- Accept either bytes or text for related_name, convert to
text.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:15>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carl Meyer <carl@…>):
In [changeset:"8aaf51f94c70e3cfcd2c75a0be1b6f55049d82d8"]:
{{{
#!CommitTicketReference repository=""
revision="8aaf51f94c70e3cfcd2c75a0be1b6f55049d82d8"
Revert "Fixed #23455 -- Forced related_name to be a unicode string during
deconstruction."
This reverts commit 45bd7b3bd9008941580c100b9fc7361e3ff3ff0d.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:14>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carl Meyer <carl@…>):
In [changeset:"f8b4cf4022aae460d7bbfb9510858baf37ae6c15"]:
{{{
#!CommitTicketReference repository=""
revision="f8b4cf4022aae460d7bbfb9510858baf37ae6c15"
[1.7.x] Revert "Fixed #23455 -- Forced related_name to be a unicode string
during deconstruction."
This reverts commit 45bd7b3bd9008941580c100b9fc7361e3ff3ff0d.
This is a backport of 8aaf51f94c70e3cfcd2c75a0be1b6f55049d82d8 from
master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:16>
Django
Reporter: brian | Owner: Markush2010
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carl Meyer <carl@…>):
In [changeset:"0a8b911582eee85ea6da0d40dd2c7b12de5a78b2"]:
{{{
#!CommitTicketReference repository=""
revision="0a8b911582eee85ea6da0d40dd2c7b12de5a78b2"
[1.7.x] Fixed #23455 -- Accept either bytes or text for related_name,
convert to text.
Backport of c72eb80d114fb5d90bd21b5549e8abd0bbd17f99 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23455#comment:17>