-
Notifications
You must be signed in to change notification settings - Fork 125
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
Pass State into the Session Backend #467
Comments
I just took a look at your proof of concept and I don't think I fully understand why you want your redis connection in the If I understand your description correctly, what you want is a connection pool for your session backend. Is there any reason why it is not possible for you to just store that in the backend but have to do so in a middleware? I'm not against passing the state to the session backend, just trying to understand why it's necessary. |
So I just haven't gone to the trouble of setting up a pool in the POC, it's just easier to create a new connection every time - but ultimately that should be handled by a proper pool. I agree, it's not ideal that to have a Redis backed session store, you have to then also create the Redis middleware, and horrible things would happen at runtime if you forgot to add the Redis middleware. But since (in my case anyway) the handlers will likely also want a Redis connection, it makes sense to use the same connection for the session and the handlers. The alternative, as you suggested to create the pool and store it in the backend would certainly be possible. I suspect though, to get a connection from the pool would probably involve a future - as it does in darkredis. In which case the backend methods ( I would be happy to have a go at getting that to work if you think it would be a better solution, or if you can think of any other better ideas? |
Ok I see so you're trying to have one redis connection pool that can be shared with the session backend and the handlers. In that case it probably makes sense to use a middleware, eventhough you'll need to ensure that the redis middleware will always run before the session middleware. |
The more I think about it, the more I think it would be useful for all the backend methods (persist_session and drop_session) to return a future anyway. It just so happens that the Redis library I have been using doesn't return a future if there are no results to return - however a lot of other libraries would. |
Yes that would probably make sense and also be more consistent. |
Would you like me to work that into the current pull request, or raise a separate issue for it? |
I think it fits into this issue/PR, so if you can include it into the current PR, that'd be cool. |
Awesome. I'll see what I can do. |
This has been implemented for most use cases in #468. However, that pull request requires returned futures to live for |
I am working on a Redis backend for storing the session.
The way to do this, I imagine, is to create a separate Middleware to manage a connection pool to Redis. This Middleware would store a connection in the State that the Session backend can use. I don't see any other way that the Redis connections could be handled, correct me if I'm wrong.
In order for the backend to use the connection it needs to have access to the State. Currently, this isn't passed in. It would be great if it could be!
I have knocked up a quick proof of concept here: https://github.com/FungusHumungus/gothamboogy/blob/master/src/redis.rs, and have a tentative pull request available - if you think this is the correct way to solve this issue. I'll submit it in a minute for your perusal.
This is probably also relevant to #69.
The text was updated successfully, but these errors were encountered: