-
Notifications
You must be signed in to change notification settings - Fork 421
feat(idempotency): Add raise_on_no_idempotency_key flag #297
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 raise_on_no_idempotency_key flag #297
Conversation
GIVEN a persistence_store with event_key_jmespath = `body` WHEN getting the hashed idempotency key with an event without a `body` key THEN raise IdempotencyValidationError
@cakepietoast @heitorlessa - you might want to debate this. For example when |
Codecov Report
@@ Coverage Diff @@
## develop #297 +/- ##
========================================
Coverage 99.71% 99.71%
========================================
Files 86 86
Lines 3143 3145 +2
Branches 149 150 +1
========================================
+ Hits 3134 3136 +2
Misses 5 5
Partials 4 4
Continue to review full report at Codecov.
|
Also if we do raise an error, should it be a new type for Idempotency key errors? |
Add `raise_on_no_idempotency_key` to enforce a idempotency key value is passed into the lambda.
@heitorlessa @cakepietoast - i added a |
@heitorlessa any concerns or changes needed for this PR ? |
I’ll look into this tomorrow morning and update here as soon as I can.
…On Tue, 2 Mar 2021 at 14:49, Michael Brewer ***@***.***> wrote:
@heitorlessa <https://github.com/heitorlessa> any concerns or changes
needed for this PR ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#297 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZPQBGI5BSLL64OLRLETILTBTUHDANCNFSM4X7V2Q3A>
.
|
Checking this now |
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.
LGTM after logic to catch iterables like empty tuple and dicts as well as doc
aws_lambda_powertools/utilities/idempotency/persistence/base.py
Outdated
Show resolved
Hide resolved
docs/utilities/idempotency.md
Outdated
### Making idempotency key required | ||
|
||
By default, events without any idempotency key don't raise any exception and just trigger a warning. | ||
If you want to ensure that at an idempotency is found, you can pass in `raise_on_no_idempotency_key` as True and an |
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.
Not sure what you mean by "If you want to ensure that at an idempotency is found" - could you simplify this to non-native English speakers, please?
Take in account for empty data structures, including non-lists iterables like tuples, dictionaries Co-authored-by: Heitor Lessa <heitor.lessa@hotmail.com>
Co-authored-by: Heitor Lessa <heitor.lessa@hotmail.com>
Merging it - Thanks a lot @michaelbrewer 🎉 |
Description of changes:
Add an option to raise an error when idempotency key value is None
GIVEN a persistence_store with event_key_jmespath =
body
and optionraise_on_no_idempotency_key
WHEN getting the hashed idempotency key with an event without a
body
key valueTHEN raise IdempotencyKeyError
Checklist
Breaking change checklist
Raising any kind of error when not passing in the data for the idemptency data is not what we currently do