-
Notifications
You must be signed in to change notification settings - Fork 404
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 helper utility #245
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #245 +/- ##
===========================================
- Coverage 99.75% 99.23% -0.52%
===========================================
Files 79 85 +6
Lines 2859 3138 +279
Branches 119 151 +32
===========================================
+ Hits 2852 3114 +262
- Misses 5 14 +9
- Partials 2 10 +8
Continue to review full report at Codecov.
|
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.
Just some comments so far.
…sistence layer modules
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.
@cakepietoast I added some updated comments.
**Processes Lambda's event in an idempotent manner** | ||
>>> from aws_lambda_powertools.utilities.idempotency import idempotent, DynamoDBPersistenceLayer | ||
>>> | ||
>>> persistence_store = DynamoDBPersistenceLayer(event_key="body", table_name="idempotency_store") | ||
>>> | ||
>>> @idempotent(persistence_store=persistence_store) | ||
>>> def handler(event, context): | ||
>>> return {"StatusCode": 200} |
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.
Question: Should we explain how it'll work under the hood here? This could help clarify how often this will call DynamoDB, what parameters it'll look at to decide if it's an idempotent request or not, etc.
Question: Should we mention how this stores the event into DynamoDB?
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.
I think if we address this it should be in the documentation. Happy to make changes to the docs based on feedback!
aws_lambda_powertools/utilities/idempotency/persistence/base.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/idempotency/persistence/base.py
Outdated
Show resolved
Hide resolved
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 for the doc. Just 2 nitpicks, but these can be changed in the future.
…don't throw the wrong error for INPROGRESS
aws_lambda_powertools/utilities/idempotency/persistence/base.py
Outdated
Show resolved
Hide resolved
aws_lambda_powertools/utilities/idempotency/persistence/base.py
Outdated
Show resolved
Hide resolved
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.
Small review changes
Co-authored-by: Michael Brewer <[email protected]>
Co-authored-by: Michael Brewer <[email protected]>
Co-authored-by: Michael Brewer <[email protected]>
Co-authored-by: Michael Brewer <[email protected]>
Issue #, if available: #218
Description of changes:
Implementing RFC #218. I'll be working together with @igorlg on this.
Todos
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.