After migrating a Django project generated with cookiecutter-django from Python 2.x to 3.x, db migration of (cookiecutter-django generated, not Django module) django.contrib.sites fails with "TypeError: attribute name must be string, not 'bytes'"

[Florian Mayer]

I'm migrating an existing Django project from Python 2.7 / Django 1.11 to Python 3.6 / Django 2.0.7 (latest stable at the time of writing).
Migrations are finicky as they (used to) be created without from __future__ import unicode_literals, therefore all strings (such as model manager names e.g. "'objects'") would be interpreted as bytestrings in Python 3.x.

Ticket 24701 is the closest issue I could find relating to this error, and it led to Django now inserting the unicode_literals import into newly created migrations. This however doesn't fix existing migrations, and while Django's migrations are doing now a decent job to write plain un-prefixed "strings" in field names, sometimes a model manager name or a more exotic migration command's argument slips through as b"explicitly prefixed bytestring".

Environment

• ​CircleCI v2 build using Python 3.6.1 (freshly upgraded from Python 2.7)
• ​requirements Django 2.0.7
• The Django project was written Django 1.9.7 to 1.11.x, and recently upgraded to Django 2.x.
• The migrations were created using Django 1.9.7 to 1.11.x
• Third party app migrations are potentially ancient
• Project generated by cookiecutter-django, which generates a stand-alone contrib.sites app (​rationale here)
• Solution: There was a stray bytestring in cookiecutter-django's contrib.sites migration.

Stack trace

​Failing CircleCI build fails at ./manage.py migrate.
Can reproduce on Ubuntu 16.04 (Python 3.6.x Anaconda) and 14.04 (Python 3.4.3).

Running migrations:
  Applying contenttypes.0001_initial... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  [... working migrations ...]
  Applying sites.0001_initial...Traceback (most recent call last):
  File "manage.py", line 12, in <module>
    execute_from_command_line(sys.argv)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/core/management/__init__.py", line 371, in execute_from_command_line
    utility.execute()
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/core/management/__init__.py", line 365, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/core/management/commands/test.py", line 26, in run_from_argv
    super().run_from_argv(argv)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/core/management/base.py", line 335, in execute
    output = self.handle(*args, **options)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/core/management/commands/test.py", line 59, in handle
    failures = test_runner.run_tests(test_labels)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/test/runner.py", line 601, in run_tests
    old_config = self.setup_databases()
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/test/runner.py", line 548, in setup_databases
    self.parallel, **kwargs
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/test/utils.py", line 176, in setup_databases
    serialize=connection.settings_dict.get('TEST', {}).get('SERIALIZE', True),
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/backends/base/creation.py", line 68, in create_test_db
    run_syncdb=True,
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/core/management/__init__.py", line 141, in call_command
    return command.execute(*args, **defaults)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/core/management/base.py", line 335, in execute
    output = self.handle(*args, **options)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/core/management/commands/migrate.py", line 200, in handle
    fake_initial=fake_initial,
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/executor.py", line 117, in migrate
    state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/executor.py", line 147, in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/executor.py", line 244, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/migration.py", line 112, in apply
    operation.state_forwards(self.app_label, project_state)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/operations/models.py", line 86, in state_forwards
    list(self.managers),
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/state.py", line 97, in add_model
    self.reload_model(app_label, model_name)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/state.py", line 158, in reload_model
    self._reload(related_models)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/state.py", line 191, in _reload
    self.apps.render_multiple(states_to_be_rendered)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/state.py", line 306, in render_multiple
    model.render(self)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/migrations/state.py", line 575, in render
    return type(self.name, bases, body)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/models/base.py", line 152, in __new__
    new_class.add_to_class(obj_name, obj)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/models/base.py", line 315, in add_to_class
    value.contribute_to_class(cls, name)
  File "/home/circleci/repo/venv/lib/python3.6/site-packages/django/db/models/manager.py", line 115, in contribute_to_class
    setattr(model, name, ManagerDescriptor(self))
TypeError: attribute name must be string, not 'bytes'

tl;dr: The name, a value coming from the migration sites.0001_initial, was interpreted by setattr(model, name, ManagerDescriptor(self)) as a bytestring.
Confusingly, this migration was inside the project, not inside django.contrib.sites.

Steps in detail

• django.contrib.sites ​migration 0001 is missing the unicode_literals statement. Under Python 3, e.g. its ​model manager name 'objects' will be interpreted as bytestring b'objects'.
• While running ./manage.py migrate, the ​django.db.models.manager L113 reads the migration's model manager name expecting a unicode string, and not defending against a bytestring.

I was able to fix my own existing migrations (created in Django 1.x) either by hand (inserting the unicode_literals import, or deleting the occasional explicit b"" prefix), or by squashing multiple migrations (where too numerous to manually edit) under Django 2.x, which inserted the unicode_literals statement into my newly created migrations.
However, this approach won't work with third party migrations without manually patching them (which won't work on e.g. CI and is brittle) or forking (creating a maintenance trail for me).

Suggested patches

• Primarily, add from __future__ import unicode_literals to django.contrib.sites' ​migration 0001.
• As a longer term fix, replace ​django.db.models.manager L113 setattr(model, name, ManagerDescriptor(self)) with setattr(model, force_text(name), ManagerDescriptor(self)) (and import force_text).
• If a bytestring is found, a Django warning should prompt the user to clean up the offending migration by adding the unicode_literals import. This would be more graceful and efficient than presenting the TypeError.

The benefit of patching db.models.manager would be that all Python 2.x-style migrations, especially of third party packages, would work in Python 3.x-based Django projects without the need to patch them.

Has anyone else encountered this bug? Happy to PR whichever fix is deemed most appropriate (bear with me, first Django bug report).

[Carlton Gibson]

Hi Florian.

This doesn't make sense quite as it is.

On Python 3 unprefixed string literals are just are unicode literals, i.e. the from __future__ import unicode_literals doesn't do anything.

Under Python 3, e.g. its ​model manager name 'objects' will be interpreted as bytestring b'objects'.

This is incorrect.

>>> 'objects'
'objects'
>>> 'objects'.encode('utf8')
b'objects'
>>> 

On Python 3, it's only when you encode the string that you get bytes.

So the question is How is this happening?

Can reproduce on Ubuntu 16.04 (Python 3.6.x Anaconda) and 14.04 (Python 3.4.3).

OK, so can you put this in a minimal project that reproduces the error? Then we've got something to look at.
(In general, the sites app is part of Django's own test suite, so those migrations are run extensively without problem, on Python 3.)

You said you're updating BOTH Django (from 1.11) and Python (from 2.7). One thought is to make sure you do the Python upgrade first. Rollback to 1.11, upgrade to Python 3, run all migrations, and make sure everything is working. Then upgrade to Django 2.x. (This is just a sanity check. It doesn't explain the error you're seeing.)

[Florian Mayer]

Hi Carlton,

thanks for the quick reply. Yes, the name string should have "just worked". Strangely, I'm getting the failure even in the circleci build, where OS and environment are off the shelf. If only I am experiencing this bug, I must have got something contained in my code base.

I'll try to build a MWE and submit findings here!

[Tim Graham]

There are similar issues like #24949 that are closed as wontfix because Django dropped support for Python 2 since Django 2.0. My guess is that you have some other migration with a bytestring that you need to fix. It doesn't make sense to patch Django to handle these mistakes at this point.

[Florian Mayer]

Tim, I agree that it is absolutely not BaseManager's responsibility to catch stray bytestrings. My rationale was that, especially with Django dropping support for Python 2, many others will also be forced to upgrade their Py 2.x/Django 1.x projects - at the latest when Django drops support for their 1.x versions. They all would benefit from at least a well worded warning pointing them into the right direction. However, let me first find the root cause so I can suggest a meaningful warning / hint.

Interim finding:

After patching BaseManager to print its arguments:

 def contribute_to_class(self, model, name):
        if not self.name:
            self.name = name
        self.model = model
        print('django.db.models.manager.BaseManager: {0}, {1}, {2}'.format(self, model, name)) # inserted print debug
        setattr(model, name, ManagerDescriptor(self))

I fount that ./manage.py migrate gets a spurious b'objects' as manager name from __fake__.Site:

django.db.models.manager.BaseManager: sites.Site.b'objects', <class '__fake__.Site'>, b'objects'

Whereas ./manage.py runserver gets an expected unicode 'objects' from 'django.contrib.sites.models.Site.

django.db.models.manager.BaseManager: sites.Site.objects, <class 'django.contrib.sites.models.Site'>, objects

This means that the actual migrations of django.contrib.sites are not broken, but the unexpected behaviour comes in (at least on my end) through the __fake__ class.
While I'm reading up on the gritty details of how Django migrations work and the inner makings of this __fake__ class, any pointers would be greatly appreciated.

[Florian Mayer]

Found the cause: The project template I've used, ​cookiecutter-django, generated a separate 'sites' app in my project directory. That one had a stray bytestring b'objects' in its migrations. Side note, cookiecutter-django was good enough to insert an ​explanation for the presence of this 'sites' app.

To conclude, I'd like to close this issue with the suggestion to include a Django warning in either django.db.models.base.ModelBase ​here if obj_name is a bytestring, or as suggested originally in django.db.models.manager ​here if name is a bytestring. Would such a warning make sense?

[Florian Mayer]

Crossreference to my issue about this bug at cookiecutter-django: ​https://github.com/pydanny/cookiecutter-django/issues/1738

[Tim Graham]

I don't think it makes sense to add a warning in Django at this point because that could only go into Django 2.2 and users will have to upgrade to Python 3 before upgrading to Django 2.0 where they will encounter this error.

[Florian Mayer]

Happy with the resolution, Tim! Thanks for the feedback!