Opened 6 years ago

Closed 6 years ago

Last modified 6 years 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 by Carlton Gibson, 6 years ago

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 6 years ago by Carlton Gibson (previous) (diff)

comment:2 by Carlton Gibson, 6 years ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

comment:3 by Aarni Koskela, 6 years ago

Owner: changed from nobody to Aarni Koskela
Status: newassigned

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

comment:4 by Aarni Koskela, 6 years ago

Has patch: set

comment:5 by Carlton Gibson, 6 years ago

Needs tests: set
Patch needs improvement: set

comment:6 by Aarni Koskela, 6 years ago

Needs tests: unset
Patch needs improvement: unset

Patch is ready and waiting for review.

comment:7 by Mariusz Felisiak, 6 years ago

Severity: NormalRelease blocker
Type: Cleanup/optimizationBug

comment:8 by Mariusz Felisiak, 6 years ago

Triage Stage: AcceptedReady for checkin

comment:9 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In efeceba5:

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

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

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