-
Notifications
You must be signed in to change notification settings - Fork 154
feat(idempotency): support for Redis
as idempotency backend
#3896
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
base: main
Are you sure you want to change the base?
feat(idempotency): support for Redis
as idempotency backend
#3896
Conversation
…n with default expiry
…ovided Redis clients
…sistenceLayer tests
…nd add `_deleteRecord` test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @arnabrahman for the amazing PR.
I have tested a build of this implementation in Lambda with a Serverless Valkey cluster and it works both with the Redis and Valkey Glide clients - which is great to see!
I have left a couple comments but the main area of change is around how we deal with the client. There are more details below, but the gist of it is that I think we should just accept a connected client
instance and let the customer decide 100% - this means removing the RedisConnection
part where we instantiate a client for them.
Besides leaving the vendor/flavor choice to the customer, we also know that there are a lot of ways to connect to a Redis-compatible cluster: for example types of certificates, auth methods, and other configurations. Because of this, it's highly likely that customers will want to bring their own configured client anyway - so let's go all in with this idea and only provide the Idempotency logic.
… clarity and consistency
… requirement in `RedisPersistenceOptions`
… handling from persistence layer
…ayer` tests and streamline client handling
…er` methods and tests
…om documentation
…CompatibleClient` interface to clarify client initialization requirements
…m `RedisPersistenceLayer`
@dreamorosi I have addressed the review comments. Quick thought: do you think it makes sense to introduce a |
Hi @arnabrahman, thank you! Yes feel free to streamline wherever applicable, especially when it comes to types. Good idea! |
… & `Redis` to extend
…fy client initialization requirements
…es` for clarity and consistency
…nt attribute handling across persistence layers
…CE_ATTRIBUTE_KEY_MAPPINGS` for clarity and consistency
…property from `DynamoDBPersistenceLayer`
|
Summary
This Pr adds support for using
Redis
as an idempotency backend. The implementation follows the same convention as the Python implementation of the same feature for powertoolsChanges
Redis
support for idempotency backendRedisPersistanceLayer
, this more or less follows the python implementationconsole
logging similar to the Python implementation, but I think in this project, there is no precedent for doing that. So not sure if it should stay or not.RedisConnection
class to handle Redis client creation@redis/client
package as default redis client, but the user can bring their own client to use it.Example:
When using default redis client
When using your own redis client
Issue number: #3183
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.