Skip to content

Conversation

@thoresuenert
Copy link
Contributor

@thoresuenert thoresuenert commented Dec 7, 2025

Close #1700

Questions:

  • Do we need actions in cleanup function when we set the expiration on the redis stored data?
  • Is the RedisSessionConfig the right way to set a prefix for session keys?
  • Do we need multiple tagged Redis setups?
  • How can we setup the Redis client to a different database for sessions in this scenario? If we want to

Laravel uses multiple configuration an treat redis as database, to separate cache from other things they configure multiple connections to different databases.
The Redis Client itself allows to set the prefix for the client as a whole. From my understanding to support that we need a tagged('session') redis client. I dont understand how to do that without touchung the kv-store.

For example RedisCacheConfig:
The cache uses the same RedisConfig as the RedisSessionConfig and the data will be written in the same database.

@thoresuenert thoresuenert marked this pull request as ready for review December 8, 2025 08:58
@innocenzi
Copy link
Member

Is the RedisSessionConfig the right way to set a prefix for session keys?

Yes!

Do we need multiple tagged Redis setups?
How can we setup the Redis client to a different database for sessions in this scenario? If we want to

Good questions, the kv-store doesn't support tagged configs yet but this should be the way to go if we want multiple database support. With a prefix I'd assume it'd be pretty safe to have sessions on the same database as the cache though, but I admit I don't know what are the best practices here

@brendt
Copy link
Member

brendt commented Dec 11, 2025

For now I don't think we should widen the scope of this PR, and just assume one Redis instance. We can add tagged Redis configs later, but let's not complicate this PR with it.

Apart from that, all looks fine!

@brendt
Copy link
Member

brendt commented Dec 11, 2025

@innocenzi did you still have an open change request?

@brendt brendt merged commit eb7150b into tempestphp:main Dec 11, 2025
74 checks passed
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.

Redis session support

4 participants