Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#24346 closed Bug (fixed)

AuthRouter docs lead to AttributeError: 'NoneType' object has no attribute '_meta' with 0002_remove_content_type_name

Reported by: Peter Schmidt Owned by: nobody
Component: Documentation Version: 1.8alpha1
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

#22583 passes model=None into allow_migrate(db, model, **hints):
https://github.com/django/django/blob/8f4877c89d0f28e289399fbb46357623a49e7eb0/django/db/migrations/operations/special.py#L174

#24099 introduced a 0002_remove_content_type_name migration which uses this code path:
https://github.com/django/django/blob/b4ac23290772e0c11379eb2dfb81c750b7052b66/django/contrib/contenttypes/migrations/0002_remove_content_type_name.py

When combined with the documented AuthRouter example:
https://docs.djangoproject.com/en/1.8/topics/db/multi-db/#an-example

This causes a stacktrace like the following in downstream projects:

$ ./manage.py test --verbosity=2
Creating test database for alias 'default' (':memory:')...
...
Running migrations:
  Rendering model states... DONE
  ...
  Applying contenttypes.0002_remove_content_type_name...Traceback (most recent call last):
  File "./manage.py", line 20, in <module>
    execute_from_command_line(sys.argv)
  File "/Users/pzrq/Projects/django/django/core/management/__init__.py", line 338, in execute_from_command_line
    utility.execute()
  File "/Users/pzrq/Projects/django/django/core/management/__init__.py", line 330, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/pzrq/Projects/django/django/core/management/commands/test.py", line 30, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/Users/pzrq/Projects/django/django/core/management/base.py", line 390, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/Users/pzrq/Projects/django/django/core/management/commands/test.py", line 74, in execute
    super(Command, self).execute(*args, **options)
  File "/Users/pzrq/Projects/django/django/core/management/base.py", line 441, in execute
    output = self.handle(*args, **options)
  File "/Users/pzrq/Projects/django/django/core/management/commands/test.py", line 90, in handle
    failures = test_runner.run_tests(test_labels)
  File "/Users/pzrq/Projects/django/django/test/runner.py", line 210, in run_tests
    old_config = self.setup_databases()
  File "/Users/pzrq/Projects/django/django/test/runner.py", line 166, in setup_databases
    **kwargs
  File "/Users/pzrq/Projects/django/django/test/runner.py", line 370, in setup_databases
    serialize=connection.settings_dict.get("TEST", {}).get("SERIALIZE", True),
  File "/Users/pzrq/Projects/django/django/db/backends/base/creation.py", line 369, in create_test_db
    test_flush=True,
  File "/Users/pzrq/Projects/django/django/core/management/__init__.py", line 120, in call_command
    return command.execute(*args, **defaults)
  File "/Users/pzrq/Projects/django/django/core/management/base.py", line 441, in execute
    output = self.handle(*args, **options)
  File "/Users/pzrq/Projects/django/django/core/management/commands/migrate.py", line 213, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/Users/pzrq/Projects/django/django/db/migrations/executor.py", line 93, in migrate
    self.apply_migration(states[migration], migration, fake=fake)
  File "/Users/pzrq/Projects/django/django/db/migrations/executor.py", line 129, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/Users/pzrq/Projects/django/django/db/migrations/migration.py", line 110, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/Users/pzrq/Projects/django/django/db/migrations/operations/special.py", line 174, in database_forwards
    if self.allowed_to_migrate(schema_editor.connection.alias, None, hints=self.hints):
  File "/Users/pzrq/Projects/django/django/db/migrations/operations/base.py", line 109, in allowed_to_migrate
    return router.allow_migrate(connection_alias, model, **(hints or {}))
  File "/Users/pzrq/Projects/django/django/db/utils.py", line 334, in allow_migrate
    allow = method(db, model, **hints)
  File "/Users/pzrq/Projects/mathspace/msproblem/core/database_routers.py", line 67, in allow_migrate
    elif model._meta.app_label in APP_LABELS:
AttributeError: 'NoneType' object has no attribute '_meta'

This is documented here:
https://docs.djangoproject.com/en/1.8/releases/1.8/#miscellaneous

The migration operations RunPython and RunSQL now call the allow_migrate() method of database routers. In these cases the model argument of allow_migrate() is set to None, so the router must properly handle this value.

So I guess the fix is to update the docs to be consistent with the previously committed changes, e.g.

class AuthRouter(object):
    ...
    def allow_migrate(self, db, model=None, **hints):
        """
        Make sure the auth app only appears in the 'auth_db'
        database.
        """
        if model is None:
            # Handle RunPython or RunSQL
            return db == 'auth_db'

        if db == 'auth_db':
            return model._meta.app_label == 'auth'
        elif model._meta.app_label == 'auth':
            return False
        return None

Will add a Github PR for that shortly.

Change History (8)

comment:2 Changed 5 years ago by Loic Bistuer

Has patch: set
Patch needs improvement: set

Returning None (no opinion) is the most sensible default in the absence of hints when model == None.

That said we should probably add hints to that content type migration in 1.8.

comment:3 Changed 5 years ago by Tim Graham

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:4 Changed 5 years ago by Loic Bistuer

This sparked #24351 and the PR on that ticket also fixes this issue.

comment:5 Changed 5 years ago by Tim Graham

Resolution: duplicate
Status: newclosed

Closing as duplicate of #24351 -- thanks for raising it.

comment:6 Changed 5 years ago by Loic Bistuer <loic.bistuer@…>

Resolution: duplicatefixed

In bed504d70bede3431a213203c13a33905d6dbf77:

Fixed #24351, #24346 -- Changed the signature of allow_migrate().

The new signature enables better support for routing RunPython and
RunSQL operations, especially w.r.t. reusable and third-party apps.

This commit also takes advantage of the deprecation cycle for the old
signature to remove the backward incompatibility introduced in #22583;
RunPython and RunSQL won't call allow_migrate() when when the router
has the old signature.

Thanks Aymeric Augustin and Tim Graham for helping shape up the patch.

Refs 22583.

comment:7 Changed 5 years ago by Loic Bistuer <loic.bistuer@…>

In 3a6c37fce452a3bbf185b5e58dd3e7c89b456b19:

[1.8.x] Fixed #24351, #24346 -- Changed the signature of allow_migrate().

The new signature enables better support for routing RunPython and
RunSQL operations, especially w.r.t. reusable and third-party apps.

This commit also takes advantage of the deprecation cycle for the old
signature to remove the backward incompatibility introduced in #22583;
RunPython and RunSQL won't call allow_migrate() when when the router
has the old signature.

Thanks Aymeric Augustin and Tim Graham for helping shape up the patch.

Refs 22583.

Conflicts:

django/db/utils.py

Backport of bed504d70bede3431a213203c13a33905d6dbf77 from master

comment:8 Changed 5 years ago by Peter Schmidt

Thanks Loïc, Aymeric, Tim and everyone else who contributed to this.

Works for me, in practice I have tended to split up apps with a large number of models in them into smaller apps which focus on one or a few closely related models, so app_label was enough, though it's really cool to see how much thought and effort goes into this area.

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