-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate omeroweb.filesessionstore #486
Conversation
Make the deprecated session store simply extend the upstream file-backend sessions store
94f6a1e
to
eff26c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With both omero.web.session_engine
set to omeroweb.filesessionstore
and unset the behaviour is as expected. Personally, I'm fine with omeroweb.filesessionstore
just inheriting for now as a backwards compatibility thing. I cannot imagine someone is depending on any specific idiosyncrasies there and if they are they can include a version of the store themselves and use that if they'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #472
This custom
SessionStore
backend was introduced almost a decade ago to address an issue with the upstream store not respecting the file expiration. As discussed in the associated issue and indicated in the original commit message, this store was expected to be a temporary workaround and removed eventually in favor of the Django built-in file backend.## Summary
This PR includes the following changes:
omeroweb.filesessionsstore.SessionStore
classDEFAULT_SESSION_ENGINE
to usedjango.contrib.sessions.backends.file
omero.web.session_engine
property documentation to list all supported engines and markomeroweb.filesessionstore
as deprecatedomeroweb.filesessionsstore.SessionStore
and make it extenddjango.contrib.sessions.backends.file.SessionStore
.746caac is arguably the biggest change in the initial proposal and could be reverted if we decided to keep the existing logic. Note in that case, the implementation would likely need to be reconciled with the upstream changes as part of the Django 4.2 upgrade effort (#480).
Impact
The impact of this change on existing OMERO.web deployments should be minimal as:
django.contrib.sessions.backends.file
omero.web.session_engine
explicitly set toomeroweb.filesessionstore
will need to be reviewedTesting
omero.web.session_engine
set toomeroweb.filesessionstore
or unset/tmp/sessionid*
on standard Linux distributions)omero.web.session_cookie_age
to30
/tmp
omero web clearsessions
and check that the session file has been deleted