Skip to content
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

Server-side sessions and unified session backends #630

Merged
merged 66 commits into from Oct 27, 2022
Merged

Server-side sessions and unified session backends #630

merged 66 commits into from Oct 27, 2022

Conversation

provinzkraut
Copy link
Member

@provinzkraut provinzkraut commented Oct 20, 2022

This is a bit WIP. Aims to implement #617

PR Checklist

  • Have you followed the guidelines in CONTRIBUTING.md?
  • Have you got 100% test coverage on new code?
  • Have you updated the prose documentation?
  • Have you updated the reference documentation?

@lgtm-com
Copy link

lgtm-com bot commented Oct 20, 2022

This pull request introduces 4 alerts when merging f19f9d2 into d11d0fb - view on LGTM.com

new alerts:

  • 3 for Module-level cyclic import
  • 1 for Unused import

@provinzkraut provinzkraut linked an issue Oct 26, 2022 that may be closed by this pull request
@provinzkraut provinzkraut marked this pull request as ready for review October 26, 2022 15:43
@provinzkraut provinzkraut changed the title [WIP] Server-side sessions and unified session backends Server-side sessions and unified session backends Oct 26, 2022
Copy link
Contributor

@Goldziher Goldziher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all, very impressive. There is still some work to be done.

pyproject.toml Outdated Show resolved Hide resolved
starlite/middleware/session/sqlalchemy_backend.py Outdated Show resolved Hide resolved
starlite/middleware/session/sqlalchemy_backend.py Outdated Show resolved Hide resolved
starlite/testing/test_client.py Outdated Show resolved Hide resolved
tests/fake_memcached.py Outdated Show resolved Hide resolved
@infohash
Copy link
Contributor

infohash commented Oct 26, 2022

Is it possible to have an extension API to use client side session as complementary? It will help a lot for extensions that manage user authentication and delegation of authority on APIs.

@provinzkraut
Copy link
Member Author

@infohash

Is it possible to have an extension API to use client side session as complementary? It will help a lot for extensions that manage user authentication and delegation of authority on APIs.

I'm still not entirely sure what you mean by this. How would complementary session backends work API wise? And what would the use case be? If the identity is being managed by a different mechanism, it could effectively also just be an arbitrary storage like redis. So why would one want to use this?

provinzkraut and others added 15 commits October 27, 2022 15:39
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
A bug was introduced as part of #626, that would lower case header values
@provinzkraut provinzkraut merged commit 55d7389 into litestar-org:main Oct 27, 2022
@provinzkraut provinzkraut deleted the server-side-sessions branch October 27, 2022 16:29
provinzkraut added a commit that referenced this pull request Oct 28, 2022
This was supported in the previous SessionMiddleware and was erroneously removed in #630. Test have been added.
provinzkraut added a commit that referenced this pull request Oct 28, 2022
* Support setting sessions explicit to `Empty`.

This was supported in the previous SessionMiddleware and was erroneously removed in #630. Test have been added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: Server-side sessions
5 participants