Opened 5 months ago

Closed 4 months ago

Last modified 4 months ago

#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:1 Changed 5 months ago by Carlton Gibson

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.

Last edited 5 months ago by Carlton Gibson (previous) (diff)

comment:2 Changed 5 months ago by Carlton Gibson

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

comment:3 Changed 5 months ago by Aarni Koskela

Owner: changed from nobody to Aarni Koskela
Status: newassigned

I'll work on a PR, then. :)

comment:4 Changed 5 months ago by Aarni Koskela

Has patch: set

comment:5 Changed 5 months ago by Carlton Gibson

Needs tests: set
Patch needs improvement: set

comment:6 Changed 4 months ago by Aarni Koskela

Needs tests: unset
Patch needs improvement: unset

Patch is ready and waiting for review.

comment:7 Changed 4 months ago by felixxm

Severity: NormalRelease blocker
Type: Cleanup/optimizationBug

comment:8 Changed 4 months ago by felixxm

Triage Stage: AcceptedReady for checkin

comment:9 Changed 4 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In efeceba5:

Fixed #30312 -- Relaxed admin check from django.contrib.sessions to SessionMiddleware subclasses.

comment:10 Changed 4 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In a4095dad:

[2.2.x] Fixed #30312 -- Relaxed admin check from django.contrib.sessions to SessionMiddleware subclasses.

Backport of efeceba589974b95b35b2e25df86498c96315518 from master

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