Position of admin in INSTALLED_APPS in 1.7+ is now more important, and may raise warnings

[Keryn Knight <django@…>]

Given the following example INSTALLED_APPS:

INSTALLED_APPS = (
    'django.contrib.contenttypes',
    'django.contrib.sites',
    'django.contrib.admin',
    'django.contrib.auth',
)

and using python -W once ...
will result in messages like:

PendingDeprecationWarning: Model class django.contrib.auth.models.User doesn't declare an explicit app_label and either isn't in an application in INSTALLED_APPS or else was imported before its application was loaded. This will no longer be supported in Django 1.9.

with no indication of how to actually fix the problem going forward. Thanks to apollo13 pushing me to trace it down, the problem is that because admin depends on both sites and auth (and contenttypes), they need to appear before it in the INSTALLED_APPS. This has historically never been a problem (and wouldn't be until 1.9, based on this warning).

The imports that trigger it (for auth, anyway) is as follows:
'django.contrib.admin' -> the __init__ does the import:
from django.contrib.admin.sites import AdminSite, site
which in turn imports:
from django.contrib.admin.forms import AdminAuthenticationForm
which in turn imports:
from django.contrib.auth.forms import AuthenticationForm
which has a hard dependency on User:
from django.contrib.auth.models import User
the only use of User in auth.forms appears to be in UserCreationForm's clean_username method, which directly relies on User being in scope, rather than get_user_model() (which itself might not be filled yet, I don't know the internals well enough to say)

The warning is triggered when django.setup() is called, which calls apps.populate(), which eventually takes the string 'django.contrib.admin' and does AppConfig.create(entry) on it, which is where the __import__ happens and the warnings are output.

For contrib.sites, the import chain is:
__init__ does:
from django.contrib.admin.sites import AdminSite, site
which imports:
from django.contrib.contenttypes import views as contenttype_views
which imports:
from django.contrib.sites.models import Site (and also from django.contrib.sites.shortcuts import get_current_site )

The issue may only be apparent if calling settings.configure and django.setup oneself, outside of the standard Django mechanisms, as this is how I've encountered it.

[Aymeric Augustin]

Great bug report! I can see three distinct problems:

• It should be possible to use django.contrib.admin without importing django.contrib.auth.models.User.
• It should be possible to use django.contrib.contenttypes without importing django.contrib.sites.models.Site.
• The error message should point to the apps docs, which should provide directions to solve this problem.

Since there's a tree of dependencies between the models modules of all applications in a Django project, it's always possible to put INSTALLED_APPS in such an order that an application always comes after its dependencies.

Unfortunately, that order also controls discovery of management commands, static files, templates, and translations. This new constraint is going to interact badly with #21018. In that ticket, I decided that apps that come first in INSTALLED_APPS have precedence for discovery, for the following reason:

I find "last app wins" more intuitive that "first app wins" but since it's more backwards compatible to standardize on "first app wins" let's do that.

It's a common pattern to create a custom app to alter the behavior of an off-the-shelf app. The custom app will usually depend on the off-the-shelf app, which means it has to come later in INSTALLED_APPS. However, it should also have precendence for discovery, which means it has to come earlier. This isn't user-friendly, to say the least.

Reversing the decision from #21018 might be the best choice, but it's a major backwards incompatibility.

[Aymeric Augustin]

Of course, as far as Django is concerned, forcing an app_label would make the warning go away, but we don't want every app to do that. We must fix the underlying issue.

[Aymeric Augustin]

Wait, my original analysis wasn't correct. The actual problem is importing models as a side effect of importing an app package, in this case django.contrib.admin.

The current implementation tolerates importing models from apps that are already loaded in the app registry, but it's an accident. It wasn't part of the original design and intent. We should consider fixing this flaw, because reordering apps isn't the solution here.

The issue will vanish if django.contrib.* packages can be imported without importing any models (except for deprecated apps, which I don't care about).

[Aymeric Augustin]

The following apps contain code in __init__.py (other than declaring default_app_config):

• admin,
• auth,
• comments, but it's deprecated,
• gis, but it doesn't import anything,
• messages, but it's easy to check that the imports do not cascade outside of the app, and the app doesn't contain models,
• sitemaps.

While the problem may not be easy to fix, at least it's limited to admin, auth, and sitemaps.

[Aymeric Augustin]

PR: ​https://github.com/django/django/pull/2228

I worked on this issue by running python -Werror manage.py runserver on a dummy project with various values of INSTALLED_APPS until I didn't run into exceptions. Since we're talking of import-time warnings, I don't know how I could write a regression test.

Moving a few imports inside functions was enough to solve the problem. Mostly, it boils down to django.contrib.admin importing tons of stuff for convenience, but this isn't a good design when there may be good reasons not to import some optional dependencies. Details follow.

auth

It depends on conttenttypes. Putting 'django.contrib.auth' first doesn't trigger a warning.

sitemaps

It may depend on sites. The docs say it's a hard dependency whereas the code is obviously written to support a subset of the features when sites isn't installed.

The first commit in my PR avoids importing the Site model in sitemaps. Then putting just 'django.contrib.sitemaps' in INSTALLED_APPS doesn't trigger a warning.

admin

It depends on auth, contenttypes, messages, sessions, and optionally sites. The default project template puts 'django.contrib.admin' first in INSTALLED_APPS. This is the worst case for the problem discussed in this ticket.

The second commit in my PR prevents importing ContentType from a variety of places and User through the authentication form, as explained above.

[Aymeric Augustin <aymeric.augustin@…>]

In f9698c43918c118a29516cbef4e23c197eb2dc25:

Suppressed the if Site._meta.installed pattern.

The purpose of this construct is to test if the django.contrib.sites
application is installed. But in Django 1.9 it will be forbidden to
import the Site model when the django.contrib.sites application isn't
installed.

No model besides Site used this pattern.

Refs #21719, #21923.

[Aymeric Augustin <aymeric.augustin@…>]

In 76ff266df1d68eb76836f159b799ed3e64979089:

Avoided importing models from django.contrib.admin.

Fixed #21923. Refs #21719.