Opened 11 years ago

Closed 11 years ago

#21831 closed Bug (fixed)

If contrib.auth is not in INSTALLED_APPS, any import (even indirectly) of anything within contrib.auth causes Django to fail to startup

Reported by: Carl Meyer Owned by: nobody
Component: Core (Other) Version: dev
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Carl Meyer)

This is due to d818e0c9b2b88276cc499974f9eee893170bf0a8 (the new checks framework) and the new stricter app-loading stuff. It also relates to #20915 (the dependency that django.test has on django.contrib.auth) and #19774 (the dependency that django.contrib.contenttypes has on django.contrib.admin and thus indirectly on django.contrib.auth).

The check_user_model check for contrib.auth is registered in django/contrib/auth/__init__.py as a side effect of importing anything from within django.contrib.auth.

Actually executing the check_user_model check when contrib.auth is not installed results in "LookupError: No installed app with label 'auth'." on the cls = apps.get_model(app_name, model_name) line.

Change History (11)

comment:1 by Carl Meyer, 11 years ago

Although it would be nice to fix #20915 at some point, this really needs to be fixed independently of that, because it's just too fragile for any import of anything from within the django.contrib.auth to be prohibited unless auth is in INSTALLED_APPS.

Admin and contenttypes may have the same problem (since they also register checks in their __init__.py), though I haven't tested yet to see if the checks they register also blow up if the app isn't installed.

I think the right solution here is for check-registration to happen in AppConfig.ready() rather than as an import-time side effect in __init__.py (yuck!); but that requires resolving the question of how to make the transition to using AppConfig for all contrib apps, which is currently under discussion.

An uglier temporary fix would be to only register the auth check (in django/contrib/auth/__init__.py) if auth is installed, or (even worse) code the check_user_model check to be robust against the app not being installed. (The latter is not bad in and of itself, but it's a poor solution to the general problem - there's no reason for that check to even be registered in the first place if auth is not installed.)

comment:2 by Russell Keith-Magee <russell@…>, 11 years ago

In 56516ade5ebde53af16ffdf97d3dce8d27a31e4b:

Refs #21831 -- Softened the TestClient dependency on contrib.auth.

This is to prevent an import of django.test causing an import (and thus
an implicit checks regisration) for an app that may not be in
INSTALLED_APPS.

Better fixes may be possible when #20915 and/or #21829 are addressed.

Thanks to @carljm for the report.

comment:3 by Russell Keith-Magee, 11 years ago

I've left this open as a ticket because the fix provided isn't ideal (and isn't tested because it's very difficult to test); at the very least it needs to be revisited before 1.7 final.

The real underlying problem isn't entirely the checks framework (at least, not directly) - the checks framework has just made the problem obvious. The underlying problem is the increased rigor on apps being in INSTALLED_APPS, combined with dependencies on contrib apps within core (and between contrib apps). django.tests depends on contrib.auth for some features (see #20915). contrib.contenttypes depends on contrib.admin (see #19774). I haven't done a complete audit, but it's possible other examples exist in Django's own code. More broadly, it's not hard to imagine similar problems exist in third party apps (especially as dependencies on contrib apps like auth).

I think the increased rigour provided by app loading is a good idea - we just need to handle the consequences.

Resolving #21829 will go a long way to improving the situation, because it will take the implicit import behaviour out of contrib/auth/__init__.py.

comment:4 by Carl Meyer, 11 years ago

Some updates here: django.contrib.contenttypes.generic imports admin stuff which imports from within django.contrib.auth; and South imports django.contrib.contenttypes.generic.

So we can broaden the list of projects that are broken until #21829 is fixed to include "any project that uses South or conttenttypes.generic, and does not use contrib.auth."

Last edited 11 years ago by Carl Meyer (previous) (diff)

comment:5 by Russell Keith-Magee, 11 years ago

I did some investigation last night - it looks like these problems can all be avoided (at least, at runtime) if we put an is_installed guard around the checks registration; e.g.:

from django.apps import apps
…

if apps.is_installed('django.contrib.auth'):
    checks.register(…)

The downside is that this breaks a couple of tests because the contrib apps aren't yet installed when the test framework imports the contrib apps, so the checks aren't registered. Ultimately, ready() is a much better solution for this, but if we can't come to a resolution, or we want an immediate fix for the alpha (and we're willing to live with a couple of known and understood test fails for the alpha), this could be an option.

comment:6 by Simon Charette, 11 years ago

For reference, the fact that django.contrib.contenttypes.generic imports admin stuff is tracked in #19774.

Last edited 11 years ago by Simon Charette (previous) (diff)

comment:7 by Carl Meyer, 11 years ago

Description: modified (diff)
Summary: Cannot run tests without contrib.auth in INSTALLED_APPSIf contrib.auth is not in INSTALLED_APPS, any import (even indirectly) of anything within contrib.auth causes Django to fail to startup

comment:8 by Russell Keith-Magee, 11 years ago

A minor clarification on the ticket title - the problem isn't just contrib.auth - it's also contrib.admin and contrib.contenttypes. If you import anything from these modules, and you don't have the corresponding app in INSTALLED_APPS, you'll have problems.

comment:9 by Aymeric Augustin, 11 years ago

2ff93e027c7b35378cc450a926bc2e4a446cacf0 must have solved at least part of this issue.

Can someone confirm if there's anything left to do, or simply close this ticket?

comment:10 by Carl Meyer, 11 years ago

It should have solved all of it I think. I can confirm and close later today.

comment:11 by Carl Meyer, 11 years ago

Resolution: fixed
Status: newclosed

I confirm that with the move of check-registrations into AppConfig.ready in the fix for #21829, this problem is resolved.

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