#30312 closed Bug (fixed)
Admin app has too hard a dependency on sessions app
Reported by: | Aarni Koskela | Owned by: | Aarni Koskela |
---|---|---|---|
Component: | contrib.admin | Version: | 2.2 |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Since #29695 (371ece2f0682e51f2f796854d3e091827a7cea63), released in 2.2, the admin app checks whether the django.contrib.sessions
app is in INSTALLED_APPS
.
Some projects may have opted to use a replacement session management app such as https://github.com/QueraTeam/django-qsessions – the admin app claims to be incompatible with such a configuration, even if it actually means "I'm going to need _some_ session management that works like django.contrib.sessions
".
Maybe it would be better to get rid of the app check and do what's being done for various middleware in the checks function anyway, e.g. something like
if not _contains_subclass('django.contrib.sessions.middleware.SessionMiddleware', settings.MIDDLEWARE): errors.append(checks.Error( "'django.contrib.sessions.middleware.SessionMiddleware' must " "be in MIDDLEWARE in order to use the admin application.", id='admin.E4XX', ))
– this would be out-of-the-box compatible with e.g. Qsessions.
The obvious workaround is to just re-add django.contrib.sessions
back into INSTALLED_APPS
which kinda works, but has the mild but unfortunate side effect of forcibly enabling the django.contrib.sessions.models.Session
model and migrations, (re-)adding a useless django_session
table into the database upon migration.
Change History (10)
comment:2 by , 6 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Bug → Cleanup/optimization |
comment:3 by , 6 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
I'll work on a PR, then. :)
comment:5 by , 6 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
comment:6 by , 6 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
Patch is ready and waiting for review.
comment:7 by , 6 years ago
Severity: | Normal → Release blocker |
---|---|
Type: | Cleanup/optimization → Bug |
comment:8 by , 6 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
System checks are designed to be silenced if not appropriate. I'm inclined to think this just such an edge-case at first glance.
But OK, yes, I guess subclasses are as legitimate here as elsewhere, so accepting as a Cleanup/Optimisation.