[Django] #22634: Making Session model and SessionStore classes more easily extendable
Django
--------------------------------------+--------------------
Reporter: sergeykolosov | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.sessions | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
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).
--
Ticket URL: <https://code.djangoproject.com/ticket/22634>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: sergeykolosov | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.sessions | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by russellm):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:1>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: assigned
Component: contrib.sessions | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by sergeykolosov):
* status: new => assigned
* owner: nobody => sergeykolosov
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:2>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by sergeykolosov):
* has_patch: 0 => 1
Comment:
Pull request: https://github.com/django/django/pull/2667
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:3>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by sergeykolosov):
PR updated (documentation and release notes).
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:4>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by sergeykolosov):
PR updated (compatibility with the latest master).
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:5>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by berkerpeksag):
* needs_better_patch: 0 => 1
* version: 1.6 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:6>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: assigned
Component: contrib.sessions | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by sergeykolosov):
* needs_better_patch: 1 => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:7>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: assigned
Component: contrib.sessions | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Left some comments for improvement on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:8>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: assigned
Component: contrib.sessions | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by sergeykolosov):
* needs_better_patch: 1 => 0
Comment:
PR updated: https://github.com/django/django/pull/4754
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:9>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: assigned
Component: contrib.sessions | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:10>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Component: contrib.sessions | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"22bb548900146832459deaefa880660c17a51516" 22bb5489]:
{{{
#!CommitTicketReference repository=""
revision="22bb548900146832459deaefa880660c17a51516"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:11>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: closed
Component: contrib.sessions | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"cc0d1eaaea40dc9d784c6974be1ce631a2087c11" cc0d1eaa]:
{{{
#!CommitTicketReference repository=""
revision="cc0d1eaaea40dc9d784c6974be1ce631a2087c11"
Refs #22634 -- Removed unneeded app_label in custom session engine
example.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:12>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: closed
Component: contrib.sessions | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"23e106bb721b3f4a41e40d9d98426e4cb2a12068" 23e106bb]:
{{{
#!CommitTicketReference repository=""
revision="23e106bb721b3f4a41e40d9d98426e4cb2a12068"
[1.10.x] Refs #22634 -- Removed unneeded app_label in custom session
engine example.
Backport of cc0d1eaaea40dc9d784c6974be1ce631a2087c11 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:13>
Django
Reporter: sergeykolosov | Owner:
Type: | sergeykolosov
Cleanup/optimization | Status: closed
Component: contrib.sessions | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"052f8499b9953e159e5b94c9afa2f243aa88ae39" 052f849]:
{{{
#!CommitTicketReference repository=""
revision="052f8499b9953e159e5b94c9afa2f243aa88ae39"
[1.9.x] Refs #22634 -- Removed unneeded app_label in custom session engine
example.
Backport of cc0d1eaaea40dc9d784c6974be1ce631a2087c11 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22634#comment:14>