-
Notifications
You must be signed in to change notification settings - Fork 407
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
feat(Idempotency): add feature for manipulating idempotent responses #4037
feat(Idempotency): add feature for manipulating idempotent responses #4037
Conversation
… an IdempotentHook functiont to be called when an idempotent response is being returned.
Still need to work on documentation - will do this next. |
Hi @walmsles! Your contributions continue to shine brightly. Access to the Idempotency operation results has long been on our backlog and I'm so happy that we will add this new feature. If you need any assistance with documentation, testing, or anything else, please don't hesitate to let me know. Thanks |
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #4037 +/- ##
===========================================
- Coverage 96.38% 96.25% -0.13%
===========================================
Files 214 216 +2
Lines 10030 10387 +357
Branches 1846 1927 +81
===========================================
+ Hits 9667 9998 +331
- Misses 259 275 +16
- Partials 104 114 +10 ☔ View full report in Codecov by Sentry. |
…er custom de-serialization
I added documentation for review. I also changed the order so that response_hook processes after the custom de-serializer if it is used. This makes the most sense to developers so the response_hook is passed an expected structure rather than a raw event format. |
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.
Hey, @walmsles! I looked at the code and ran some tests, and everything looked good to me at first glance! I left some comments for us to discuss before a final review. Thank you so much!
Hey @walmsles! I've updated the documentation and example. I believe it's ready for the final review now. I'll also be updating the issue with the decisions regarding exception/error handling, including why we allow customers to handle their own exceptions and why we fail in case of an exception. |
I love the being a Good Citizen sections! That makes so much more sense - I knew it was missing something. |
Reviewing now |
Quality Gate passedIssues Measures |
@rubenfonseca ready to one more review. |
Great job @walmsles @leandrodamascena pulling this off! |
Issue number: #2164
Summary
I have added the capability for developers to provide a response hook, which is called when the idempotency handler finds a valid idempotent response.
Changes
Addition of "response_hook" in IdempotentConfig class to enable developers to register a repsonse_hook
User experience
Developer experience and implementation
Response hooks
We are introducing support for hooks in the Idempotency utility. Customers can now create a function and pass it as the hook in the IdempotencyConfig. This hook will be invoked every time a request is idempotent. If the request is not idempotent, the hook will not execute.
Manipulating response
This hook will be able to access both: the response from the invocation and the
DataRecord
from the Idempotency Store. This means customers can inject new keys into the response or utilize attributes from the DataRecord to log information or create metrics, for example.Persistent Store
This hook is compatible with any Persistent Store. Currently, we support DynamoDB and Redis for this implementation.
Handling exceptions
Customers are responsible for handling exceptions when using Response hooks. During the implementation of this new feature, we considered the possibility of implementing soft failure or raising warnings to ignore hook execution. For instance, this approach could prevent customers from having exceptions in the hook, thereby avoiding potential malfunctions in the Lambda function execution. However, we opted against this approach because it could result in unexpected behaviors and potentially breaking the contract that customers expect for
Response hooks
. Let me provide some context with an example.Consider a scenario where a customer creates a hook to inject a specific key into the response when the operation is idempotent and wants to use it to trigger an alert for third-party integration. The code might appear like this:
If we choose to ignore the hook execution exception and only raise a warning instead of propagating the exception, customers may encounter difficulties in debugging. They may struggle to understand why the
if process_order["x-idempotent-key"] is not None:
condition is failing and why thex-idempotent-key
doesn't exist in the response.Documentation
The documentation will be updated to inform customers on the best practices for using Response hooks. Additionally, it will feature an example demonstrating how to utilize Response hooks and effectively handle exceptions.
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
No, not a breaking change. Configuration is optional and implementation checked for existence of the hookRFC issue number: 2164
Checklist:
Acknowledgment
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.