Opened 10 years ago

Closed 8 years ago

Last modified 8 years ago

#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 of SessionStore class, removing hardcoded usages;
  • make key_prefix a property of SessionStore, defaulting to KEY_PREFIX;
  • (anything else to make building a custom backend based on Django's one easier).

Change History (14)

comment:1 Changed 10 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

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.

comment:2 Changed 10 years ago by Sergey Kolosov

Owner: changed from nobody to Sergey Kolosov
Status: newassigned

comment:3 Changed 10 years ago by Sergey Kolosov

Has patch: set

comment:4 Changed 10 years ago by Sergey Kolosov

PR updated (documentation and release notes).

comment:5 Changed 9 years ago by Sergey Kolosov

PR updated (compatibility with the latest master).

comment:6 Changed 9 years ago by Berker Peksag

Patch needs improvement: set
Version: 1.6master

comment:7 Changed 9 years ago by Sergey Kolosov

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 Changed 8 years ago by Tim Graham

Patch needs improvement: set

Left some comments for improvement on the PR.

comment:9 Changed 8 years ago by Sergey Kolosov

Patch needs improvement: unset

comment:10 Changed 8 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:11 Changed 8 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 22bb5489:

Fixed #22634 -- Made the database-backed session backends more extensible.

Introduced an AbstractBaseSession model and hooks providing the option
of overriding the model class used by the session store and the session
store class used by the model.

comment:12 Changed 8 years ago by Tim Graham <timograham@…>

In cc0d1eaa:

Refs #22634 -- Removed unneeded app_label in custom session engine example.

comment:13 Changed 8 years ago by Tim Graham <timograham@…>

In 23e106bb:

[1.10.x] Refs #22634 -- Removed unneeded app_label in custom session engine example.

Backport of cc0d1eaaea40dc9d784c6974be1ce631a2087c11 from master

comment:14 Changed 8 years ago by Tim Graham <timograham@…>

In 052f849:

[1.9.x] Refs #22634 -- Removed unneeded app_label in custom session engine example.

Backport of cc0d1eaaea40dc9d784c6974be1ce631a2087c11 from master

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