#29508 closed Cleanup/optimization (invalid)
Simplify overriding `login_form` in AdminSite
Reported by: | Rémi Lapeyre | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | 2.0 |
Severity: | Normal | Keywords: | |
Cc: | Carlton Gibson | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
When setting login_form
of AdminSite
to a subclass of AdminAuthenticationForm an AppRegistryNotReady exception gets raised as documented in https://github.com/django/django/blob/master/django/contrib/admin/sites.py#L377.
This problem makes it harder to customize the administration site as it requires to override the whole login
method of AdminSite to bypass it.
The attached patch use import_string
to let users specify the class to use as a string and import it at runtime.
Change History (7)
comment:1 by , 7 years ago
Cc: | added |
---|---|
Resolution: | → invalid |
Status: | new → closed |
comment:2 by , 7 years ago
Hi, I made a small example project at https://github.com/remilapeyre/bug_29508
When starting django, we get the following error:
➜ bug_29508 git:(master) pipenv run bug_29508/manage.py runserver Unhandled exception in thread started by <function check_errors.<locals>.wrapper at 0x1023eb0d0> Traceback (most recent call last): File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/utils/autoreload.py", line 225, in wrapper fn(*args, **kwargs) File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/core/management/commands/runserver.py", line 112, in inner_run autoreload.raise_last_exception() File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/utils/autoreload.py", line 248, in raise_last_exception raise _exception[1] File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/core/management/__init__.py", line 327, in execute autoreload.check_errors(django.setup)() File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/utils/autoreload.py", line 225, in wrapper fn(*args, **kwargs) File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/__init__.py", line 24, in setup apps.populate(settings.INSTALLED_APPS) File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/apps/registry.py", line 89, in populate app_config = AppConfig.create(entry) File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/apps/config.py", line 90, in create module = import_module(entry) File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/importlib/__init__.py", line 126, in import_module return _bootstrap._gcd_import(name[level:], package, level) File "<frozen importlib._bootstrap>", line 994, in _gcd_import File "<frozen importlib._bootstrap>", line 971, in _find_and_load File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 665, in _load_unlocked File "<frozen importlib._bootstrap_external>", line 678, in exec_module File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed File "/Users/remi/home/src/bug_29508/bug_29508/myadmin/__init__.py", line 1, in <module> from .sites import site File "/Users/remi/home/src/bug_29508/bug_29508/myadmin/sites.py", line 2, in <module> from .forms import AdminAuthenticationForm File "/Users/remi/home/src/bug_29508/bug_29508/myadmin/forms.py", line 2, in <module> from django.contrib.admin import forms as admin_forms File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/contrib/admin/forms.py", line 2, in <module> from django.contrib.auth.forms import AuthenticationForm, PasswordChangeForm File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/contrib/auth/forms.py", line 10, in <module> from django.contrib.auth.models import User File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/contrib/auth/models.py", line 2, in <module> from django.contrib.auth.base_user import AbstractBaseUser, BaseUserManager File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/contrib/auth/base_user.py", line 47, in <module> class AbstractBaseUser(models.Model): File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/db/models/base.py", line 100, in __new__ app_config = apps.get_containing_app_config(module) File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/apps/registry.py", line 244, in get_containing_app_config self.check_apps_ready() File "/Users/remi/.local/share/virtualenvs/bug_29508-NXp2shP4/lib/python3.6/site-packages/django/apps/registry.py", line 127, in check_apps_ready raise AppRegistryNotReady("Apps aren't loaded yet.") django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.
The culprit is https://github.com/remilapeyre/bug_29508/blob/master/bug_29508/myadmin/__init__.py#L1 that will try to load django.contrib.auth.models.User through django.contrib.auth.forms.AuthenticationForm to add a placeholder in the Authentication form before the apps are registred.
Of course, we could remove the singleton from there but the customized admin would then be less friendly to use than the vanilla one as you could not do myadmin.site.register(MyModel)
like we usually can.
I'm not sure why this behavior does not happen in the current tests, maybe apps are already registred when the override of the settings occurs.
If there is anything wrong in the example project that I missed, please point me to it but it's Django's starter project with just a few modifications (https://github.com/remilapeyre/bug_29508/commit/ef75a02aa4eee09062106801502ba8129d8754b1).
comment:3 by , 7 years ago
Resolution: | invalid |
---|---|
Status: | closed → new |
comment:4 by , 7 years ago
Right, you can't load any models in an app's __init__.py
. You could empty that file and use references like this:
from myadmin.sites import site from django.urls import path urlpatterns = [ path('admin/', site.urls), ]
comment:5 by , 7 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:6 by , 7 years ago
Right, you can't load any models in an app's init.py. You could empty that file and use references like this
Yes, I understand that but isn't it weird that we can't get the exact same API when extending the admin? It seems more dev-friendly and less confusing to be able to replace admin.site.register(MyModel)
by myadmin.site.register(MyModel)
even if not everybody do this.
The proposed patch does not increase the complexity of AdminSite but allows for this usage.
comment:7 by , 7 years ago
Hi Rémi.
I don't think there's anything you can't do here. The only constraint is that you don't use models at import time.
If you keep your register
calls in admin.py
and route the URLs as Tim suggests, keeping your __init__.py
files clean, you will be able to do what you want avoiding the AppRegistryNotReady
exception.
I'm afraid I'm not seeing how this comes up at all.
The expected usage is to subclass
AdminSite
and to setlogin_form
in your class definition, importing your custom form class there.If you do this, there is no need to override
login()
.This is demonstrated by the test suite:
And exercised by `tests.admin_views.tests.CustomModelAdminTest.test_custom_admin_site_login_form()`.
This was introduced in cc64fb5c4b4315a4ad66e21458e27ece57266847 for #8342, which was part of Django v1.3.
If you can provide information demonstrating an issue here we can reassess but pending that I'm going to close.