#22634 closed Cleanup/optimization (fixed)
Making Session model and SessionStore classes more easily extendable
Reported by: | Sergey Kolosov | Owned by: | Sergey Kolosov |
---|---|---|---|
Component: | contrib.sessions | Version: | dev |
Severity: | Normal | 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
While developing a feature the involves adding an extra database field to the Session
model (thus creating a new session backend based on Django's one), I faced the need to copy almost everything from models.py
, backends/db.py
and backends/cached_db.py
.
The reason why one have to do that, is the way that Session
model is imported and used inside backends/db.py
and backends/cached_db.py
, and the way SessionStore
class imported and used inside models.py
.
Since the custom backend is a part of a shared package, which is intended to be used with multiple versions of Django, it's challenging to do that given the above.
I suggest the following changes:
- extract an abstract base model from
Session
; - make
Session
class a property ofSessionStore
class, removing hardcoded usages; - make
key_prefix
a property ofSessionStore
, defaulting toKEY_PREFIX
; - (anything else to make building a custom backend based on Django's one easier).
Change History (14)
comment:1 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 11 years ago
Has patch: | set |
---|
Pull request: https://github.com/django/django/pull/2667
comment:6 by , 10 years ago
Patch needs improvement: | set |
---|---|
Version: | 1.6 → master |
comment:7 by , 10 years ago
Patch needs improvement: | unset |
---|
An new (updated) PR: https://github.com/django/django/pull/4754
This one also addresses recent Django changes in models/migrations, namely not being able to import models.py of an application which is not in INSTALLED_APPS.
comment:8 by , 9 years ago
Patch needs improvement: | set |
---|
Left some comments for improvement on the PR.
comment:9 by , 9 years ago
Patch needs improvement: | unset |
---|
PR updated: https://github.com/django/django/pull/4754
comment:10 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Sounds good to me - the sessions framework is designed to be extensible through backends, so we should be doing whatever we can to maximise reuse.