#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
Sessionclass a property ofSessionStoreclass, removing hardcoded usages; - make
key_prefixa 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 , 11 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 , 10 years ago
| Patch needs improvement: | set |
|---|
Left some comments for improvement on the PR.
comment:9 by , 10 years ago
| Patch needs improvement: | unset |
|---|
PR updated: https://github.com/django/django/pull/4754
comment:10 by , 10 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.