Allow static resources to bypass opening sessions #254
Comments
|
Hi @markhobson thanks for the issue. In the latest version there should not be anything set to storage unless you are modifying the session or have SESSION_REFRESH_EACH_REQUEST set to true. Can you confirm you aren't setting the session at all like was occurring in #232 ? Are you using a proxy or hosting service to cache your static files? That would normally preclude this issue. However, regarding not opening the session at all, this is an interesting proposal if there is a use case. I wonder if there is a better way though than providing urls, which would add complexity for a developer, rather than simplifying. Something that would only trigger on read or write to session object. One idea is re-working how flask works with sessions. Rather than opening a session with each request, the session is filled with an empty session object and the object itself calls the open_session method by overwriting the get/set method and checking if previously accessed or modified. This could be worth experimenting, but it's a pretty fundamental change. |
|
Hi @Lxstr, thanks for the considered reply. I'm not setting the session in each request but I was unaware of Turning that flag off, it does indeed no longer update the session in the storage backend. It does still open the session on every request which queries the database. The reason I'm trying to minimise hits to the database is to work around #251 - this is an issue running locally for development when using an in-memory SQLite database, hence why a proxy wouldn't be useful here. Is what you're proposing here a lazy session? One that does nothing until it is get or set. This could be implemented as a decorating You might also be interested in a few other calls to avoid opening a session that I read: |
|
I had a go at the lazy session pattern. This seems to do the trick: from typing import Any, Callable, Iterator
from flask import Flask, Request, Response
from flask.sessions import SessionInterface, SessionMixin
class LazySessionInterface(SessionInterface):
def __init__(self, delegate: SessionInterface):
self._delegate = delegate
def open_session(self, app: Flask, request: Request) -> SessionMixin | None:
def open_session_delegate() -> SessionMixin | None:
return self._delegate.open_session(app, request)
return LazySession(open_session_delegate)
def save_session(self, app: Flask, session: SessionMixin, response: Response) -> None:
if not isinstance(session, LazySession):
raise TypeError(f"session must be a LazySession: {session}")
if not session.is_open:
return
return self._delegate.save_session(app, session.delegate, response)
class LazySession(SessionMixin):
def __init__(self, open_session: Callable[[], SessionMixin | None]):
self._open_session = open_session
self._delegate: SessionMixin | None = None
def __setitem__(self, key: str, value: Any) -> None:
return self.delegate.__setitem__(key, value)
def __delitem__(self, key: str) -> None:
return self.delegate.__delitem__(key)
def __getitem__(self, key: str) -> Any:
return self.delegate.__getitem__(key)
def __len__(self) -> int:
return self.delegate.__len__()
def __iter__(self) -> Iterator[str]:
return self.delegate.__iter__()
@property
def delegate(self) -> SessionMixin:
if self._delegate is None:
self._delegate = self._open_session()
return self._delegate
@property
def is_open(self) -> bool:
return self._delegate is not NoneYou can try it out in markhobson/flask-session-issue from #251 with: ...
def create_app():
...
Session(app)
app.session_interface = LazySessionInterface(app.session_interface)
...Before these changes, putting breakpoints on Using Finally, combining that with |
|
Hi @markhobson thanks for the great effort on this. I'm really liking it. I still I have a few PRs to merge but this could be good to add for 1.0. I can see that there would be use for this. We still would have some implementation details to hash out and ask the core team about it. How would it be best to implement, with a flag or built in directly? There's some downside to each. On one hand adding more flags means there's a lot of settings and have to change defaults to get the ideal performance. However, directly built in might be too large a change even for a major bump. A third option might be replacing SESSION_REFRESH_EACH_REQUEST with SESSION_LAZY and result in either your first or your final example. @ThiefMaster @davidism I'd appreciate your thoughts on this one. Is this code a valid/readable approach? Could you see any unintended side effects? |
|
You can find my thoughts in pallets/flask#5491. The author didn't really address any of it. You brought up the same main point: static files should be served by the HTTP server, at which point this is no longer needed.
|
Requests for static resources currently cause a session to be opened, which can be costly as the session expiry date is updated in the storage backend. Consider adding a configuration key to bypass opening sessions for static resources.
A workaround is to use a custom session interface, for example:
Configured with:
The text was updated successfully, but these errors were encountered: