Opened 10 years ago

Closed 7 years ago

#22872 closed Cleanup/optimization (wontfix)

Backwards incompatible change: Can't proxy User model: RuntimeError: App registry isn't ready yet.

Reported by: Jon Dufresne Owned by: nobody
Component: Documentation Version: 1.7
Severity: Normal Keywords:
Cc: app-loading Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

My application creates proxies of the user model to add custom methods that are useful in certain contexts. There is an attempt to do this in a AUTH_USER_MODEL agnostic way. This fails with the latest beta version of 1.7.

Starting from a fresh install and project I created the following files in directory myapp.

myapp/models.py

from django.contrib.auth import get_user_model


class MyUser(get_user_model()):
    def my_method(self):
        return 'foo'

    class Meta:
        proxy = True

myapp/tests.py

from django.test import TestCase
from myapp.models import MyUser


class MyTestCase(TestCase):
    def test_my_user(self):
        user = MyUser()
        self.assertTrue(user)

Django 1.7 beta 2

$ python manage.py test
Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/jon/djtest/venv/lib/python2.7/site-packages/django/core/management/__init__.py", line 427, in execute_from_command_line
    utility.execute()
  File "/home/jon/djtest/venv/lib/python2.7/site-packages/django/core/management/__init__.py", line 391, in execute
    django.setup()
  File "/home/jon/djtest/venv/lib/python2.7/site-packages/django/__init__.py", line 21, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/home/jon/djtest/venv/lib/python2.7/site-packages/django/apps/registry.py", line 106, in populate
    app_config.import_models(all_models)
  File "/home/jon/djtest/venv/lib/python2.7/site-packages/django/apps/config.py", line 190, in import_models
    self.models_module = import_module(models_module_name)
  File "/usr/lib64/python2.7/importlib/__init__.py", line 37, in import_module
    __import__(name)
  File "/home/jon/djtest/djtest/myapp/models.py", line 4, in <module>
    class MyUser(get_user_model()):
  File "/home/jon/djtest/venv/lib/python2.7/site-packages/django/contrib/auth/__init__.py", line 136, in get_user_model
    return django_apps.get_model(settings.AUTH_USER_MODEL)
  File "/home/jon/djtest/venv/lib/python2.7/site-packages/django/apps/registry.py", line 187, in get_model
    self.check_ready()
  File "/home/jon/djtest/venv/lib/python2.7/site-packages/django/apps/registry.py", line 119, in check_ready
    raise RuntimeError("App registry isn't ready yet.")
RuntimeError: App registry isn't ready yet.

Django 1.6.5

$ python manage.py test
Creating test database for alias 'default'...
.
----------------------------------------------------------------------
Ran 1 test in 0.001s

OK
Destroying test database for alias 'default'...

If there is a better (or forward compatible) way proxy an unknown User model, I'd be happy to hear about it.

Change History (14)

comment:1 by Aymeric Augustin, 10 years ago

As part of the app-loading refactor, it's expected that calling get_user_model() at compile-time fail with this message.

The best idea I have is to create an AppConfig for your app and create the model in its ready() method, then add it to the module.

I suspect that many non-declarative models will require this pattern.

I'm leaving the ticket open for now in case someone has other ideas which may involve changes in Django.

in reply to:  1 ; comment:2 by Jon Dufresne, 10 years ago

Replying to aaugustin:

The best idea I have is to create an AppConfig for your app and create the model in its ready() method, then add it to the module.

How would one go about importing this model for use elsewhere (other models, forms, views, etc.)? These references are often used at compile time such as model forms and generic views. It seems, importing the proxy model would never work at compile time. Every function that referenced the proxy model directly would need to import it at the top of the function so as to delay the import. Or, I guess, implement another get_user_model() style function and sprinkle it all over.

in reply to:  2 comment:3 by Aymeric Augustin, 10 years ago

Indeed, my suggestion doesn't work.

Replying to jdufresne:

Or, I guess, implement another get_user_model() style function and sprinkle it all over.

I'm not sure that helps, because you still need to resolve the model at compile time.

The only thing that works at compile time currently is settings.AUTH_USER_MODEL. It can be used a an FK target because Django specifically delays resolution of FKs until the target model is imported (search for class_prepared in the code).

comment:4 by Tim Graham, 10 years ago

Cc: app-loading added
Component: UncategorizedDocumentation
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

I guess we can accept this as a documentation improvement?

comment:5 by Phillip Marshall, 10 years ago

I also have an app that uses a user proxy (also with class MyUser(get_user_model()):), which i'm porting up to 1.7. Looking through the docs it seemed like using apps.get_model('app.model') (equivalent to apps.get_model('app', 'model')) was they way to go about proxying to the user model:

from django.apps import apps
from django.conf import settings

class MyUser(apps.get_model(settings.AUTH_USER_MODEL)):
    class Meta:
        proxy = True

but this raises "django.core.exceptions.AppRegistryNotReady: Models aren't loaded yet."

It looks like this can be worked around by using the longer apps.get_app_config('app').get_model('model'). I'm not sure if this is intended behavior or a bug:

user_app, user_model = settings.AUTH_USER_MODEL.split('.')

class MyUser(apps.get_app_config(user_app).get_model(user_model)):
    ...

Some caveats: this only works if your model is after the base user model in INSTALLED_APPS, and as long as AUTH_USER_MODEL only has one dot.

I suppose circumvents the whole point of the new app system, so that plus the caveats probably makes it a bad idea to use. Rather than the above, I think a new get_user_model() should be made, or a more clear way of doing this should be documented.

The above stuff was tested on 1.7c2 and the latest stable/1.7.x as of writing, http://github.com/django/django/commit/0f44d9f9b1. - I also haven't checked any of the implications of this with migrations.

Thanks

comment:6 by Tim Graham, 9 years ago

Version: 1.7-beta-21.7

comment:7 by Tim Graham, 9 years ago

I noticed some ugly code in django-cms to work around this issue. That code is currently inspecting apps.all_models to get the user model before the apps registry is ready.

It seems to me as long as the application declaring the custom user model appears in INSTALLED_APPS before the application that calls get_user_model() there shouldn't be a problem with this technique. Am I wrong?

comment:8 by Aymeric Augustin, 9 years ago

During the app-loading refactor, we agreed that we didn't want to introduce constraints on the ordering of INSTALLED_APPS other than precedence for finding templates, translations, etc. Otherwise you could very quickly end up with a set of incompatible contraints.

I'm -1 on making the ability to import models conditional on a properly ordered INSTALLED_APPS. Debugging import loops during models loading is bad enough already (albeit better since 1.7).

If you want to get the user model anyway, apps.all_models or apps.get_registered_model both "work", in the sense that they'll find the model if it was already registered. The latter looks more like an API than the former but there's a ticket about removing it.

comment:9 by Aymeric Augustin, 9 years ago

One solution may be to introduce the equivalent of django-oscar's get_model() function and change get_user_model() to use it.

I don't think it reintroduces any of the problems app-loading eliminated, although that's hard to prove.

comment:10 by Simon Charette, 8 years ago

The introduction of an equivalent shim look reasonable. Do you have an API in mind?

comment:11 by Ivan Ven Osdel, 8 years ago

I feel like this breaking change should have been added to the Django 1.7, 1.8 and 1.9 breaking changes section. Is it too late for that?

comment:12 by Tim Graham, 8 years ago

No, feel free to submit a patch. It would only be added to the 1.7 release notes, not all subsequent versions.

in reply to:  12 comment:13 by Ivan Ven Osdel, 8 years ago

Replying to timgraham:

No, feel free to submit a patch. It would only be added to the 1.7 release notes, not all subsequent versions.

Nevermind on me updating the docs. I have run into issues with app loading and model inheritence but I am not sure they are related to this issue and they need to be better pinned down before I can do a proper docs update. https://github.com/django/django/pull/5784

Last edited 8 years ago by Ivan Ven Osdel (previous) (diff)

comment:14 by Tim Graham, 7 years ago

Resolution: wontfix
Status: newclosed

Closing since 1.7 is old and no one has bothered to write a patch for the release notes. #25966 is for allowing using get_user_model() at the module level.

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