Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#19753 closed Uncategorized (needsinfo)

Custom user system shouldn't be dependent on all installed apps

Reported by: abraham@… Owned by: nobody
Component: contrib.auth Version: 1.5-beta-1
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

function calls: 'get_user_model' => 'get_model' => _populate

_populate loads all installed apps. It seems like get_user_model should only rely on the app defining the custom user model.

Change History (14)

comment:1 by Aymeric Augustin, 11 years ago

Resolution: needsinfo
Status: newclosed

That's the behavior of Django's app loading mechanism. Improving it is a huge endeavor. See #3591.

You didn't describe any concrete drawback that could justify this amount of work.

comment:2 by abraham@…, 11 years ago

I don't think get_model should be changed... rather... get_user_model should use something else. Basically, get_user_model can't be called when creating models (IE: Referencing the user object in another model) .

comment:3 by Simon Charette, 11 years ago

I know it can't be called from the application containing the swapped for user model

# myapp/models.py
from django.contrib.auth import get_user_model
from django.contrib.auth.models import AbstractUser

class CustomUser(AbstractUser):
    pass

get_user_model() # Will fail because the app "myapp" is not installed yet, it's being installed.

But that's not really an issue since you can refer to it directly.

The main problem is that every third-party that has to call get_user_model (signal attaching for example) must be placed after 'myapp' in your INSTALLED_APPS.

As pointed out by @aaugustin this a limitation of the current app loading mechanism.

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

comment:4 by anonymous, 11 years ago

Right. I ran into this problem when updating django-guardian for my own purposes. Any third-party module that is dependent on a custom User model will have problems. I see what you guys are saying about the app loading mechanism. Is there no way to lazily load apps?

comment:5 by abraham@…, 11 years ago

I agree with what both of the guys above me are saying. I too ran into this problem when updating a third-party module. It will make dependency on a custom user model very difficult.

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

@anon - the problem isn't "Any third party module that is dependent on a custom User". You can write a module that has foreign keys to the custom User, for example. What you can't do is have module level initialization logic that depends on the pluggable user. The docs already reference one specific case where this is an issue -- signals -- but if there are other common use cases for this, then perhaps they need to be documented. As @charettes and @aaugustin have pointed out, this is a fundamental problem with the load sequence for Django apps, which should be addressed by #3591.

So - What's the use case (beyond signals) that requires a module-level reference to the swappable User model?

comment:7 by anonymous, 11 years ago

The error I'm seeing is with the admin module and django-guardian, which supposedly has Django 1.5 support. It has a foreign key reference to the custom user model somewhere, but the very fact that it programatically chooses which User model to use seems to be the problem. I'll get the trace for you guys later tonight. It may very well be something I am doing wrong.

comment:8 by abraham@…, 11 years ago

Hey anonymous... I think we're running into the same issue... here's my guardian setup error:

ERROR 2013-02-06 01:26:23,372 mlogging 3343 139807877195520 Traceback (most recent call last):
  File "/site-packages/django/core/handlers/wsgi.py", line 255, in __call__
    response = self.get_response(request)
  File "/site-packages/django/core/handlers/base.py", line 178, in get_response
    response = self.handle_uncaught_exception(request, resolver, sys.exc_info())
  File "/site-packages/django/core/handlers/base.py", line 220, in handle_uncaught_exception
    if resolver.urlconf_module is None:
  File "/site-packages/django/core/urlresolvers.py", line 342, in urlconf_module
    self._urlconf_module = import_module(self.urlconf_name)
  File "/site-packages/django/utils/importlib.py", line 35, in import_module
    __import__(name)
  File "./urls.py", line 4, in <module>
    admin.autodiscover()
  File "/site-packages/django/contrib/admin/__init__.py", line 29, in autodiscover
    import_module('%s.admin' % app)
  File "/site-packages/django/utils/importlib.py", line 35, in import_module
    __import__(name)
  File "/site-packages/guardian/admin.py", line 13, in <module>
    from guardian.forms import UserObjectPermissionsForm
  File "/site-packages/guardian/forms.py", line 4, in <module>
    from guardian.shortcuts import assign
  File "/site-packages/guardian/shortcuts.py", line 11, in <module>
    from guardian.core import ObjectPermissionChecker
  File "/site-packages/guardian/core.py", line 6, in <module>
    from guardian.utils import get_identity
  File "/site-packages/guardian/utils.py", line 19, in <module>
    from guardian.compat import AnonymousUser
  File "/site-packages/guardian/compat.py", line 12, in <module>
    User = get_user_model()
  File "/site-packages/django/contrib/auth/__init__.py", line 125, in get_user_model
    user_model = get_model(app_label, model_name)
  File "/site-packages/django/db/models/loading.py", line 230, in get_model
    self._populate()
  File "/site-packages/django/db/models/loading.py", line 75, in _populate
    self.load_app(app_name)
  File "/site-packages/django/db/models/loading.py", line 96, in load_app
    models = import_module('.models', app_name)
  File "/site-packages/django/utils/importlib.py", line 35, in import_module
    __import__(name)
  File "/site-packages/guardian/models.py", line 11, in <module>
    from guardian.compat import User
ImportError: cannot import name User

comment:9 by Simon Charette, 11 years ago

This `ForeignKey` should reference getattr(django.conf.settings, 'AUTH_USER_MODEL', 'auth.User'). This looks like the culprit.

comment:10 by abraham@…, 11 years ago

I'll give this a shot after work!

comment:11 by abraham@…, 11 years ago

Just tried it.

It seems like it has nothing to do with the foreign key reference. More to do with the importing using get_user_model() and inclusion of admin module (autodiscover maybe?).

comment:12 by anonymous, 11 years ago

To be more clear... it looks like admin.autodiscover() is executed before the models are imported. The guardian/models.py and guardian/admin.py. files are dependent on compat.py. I guess get_user_model() should not be called when importing any models.py files.

comment:13 by daniel, 11 years ago

Creating circular references is all too easy. If you want to avoid a lot of pain going forward, you're going to want to take a serious look at this issue.

comment:14 by Fernando Rocha <fernandogrd@…>, 11 years ago

This is a very annoying edge case.
It is hard to debug and third party apps are making the mistake of using get_user_model with ManyToManyField (even with warning in docs).

I would prefer that get_user_model did not depend on INSTALLED_APPS and deal with problems caused by defining AUTH_USER_MODEL without app_label in INSTALLED_APPS.

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