allow custom redis client for a more flexible setup #1194
Conversation
| self.state_manager.setup(state=self.state) | ||
|
|
||
| self.state_manager.setup( | ||
| state=self.state, redis_client=kwargs.get("redis_client", None) |
There was a problem hiding this comment.
Should we add the redis_client to the pcconfig instead? I think it makes more sense as a config field there.
There was a problem hiding this comment.
it may be a good idea to pass cache_client instead of redis_client . To get rid of the direct dependency on Redis and use any available cache server. For example: RedisCache , MemoryCache , DummyCache .
There was a problem hiding this comment.
Yes agreed, we should abstract that part out
There was a problem hiding this comment.
I can finish this PR, if there are no objections. @picklelo @sheldonchiu
There was a problem hiding this comment.
I can finish this PR, if there are no objections. @picklelo @sheldonchiu
sure, but my understand is that pcconfig should be a static setting, and adding this could break the init method
There was a problem hiding this comment.
I can finish this PR, if there are no objections. @picklelo @sheldonchiu
sure, but my understand is that
pcconfigshould be a static setting, and adding this could break the init method
I'll think of a better way and propose a solution
There was a problem hiding this comment.
Please check #1676, there has been a major refactoring to how the StateManager works to accommodate alternative backing stores for state. Specifically, StateManager has become an abstract class with StateManagerMemory and StateManagerRedis implementing it. Also the redis implementation has moved to asyncio only to interop with the uvicorn ASGI server while providing non-blocking locking of state.
If you still want to work on this issue, highly advise to either wait till that PR merges, or base your work upon it.
There was a problem hiding this comment.
@ozeranskii heads up that #1676 has merged to main.
There was a problem hiding this comment.
@masenf it seems this PR is no longer relevant and can be closed?
|
Closing PR. Future contributions in this area should focus on implementing a StateManager subclass that follows the necessary protocol. |
All Submissions:
Type of change
New Feature Submission:
Allow users to bring their own Redis client if they need a more complex connection setup