Skip to content
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

docs(idempotency): fix misleading idempotent examples #661

Merged
merged 5 commits into from
Sep 28, 2021
Merged

docs(idempotency): fix misleading idempotent examples #661

merged 5 commits into from
Sep 28, 2021

Conversation

walmsles
Copy link
Contributor

@walmsles walmsles commented Aug 28, 2021

Issue #, if available: #657

Description of changes:

Updated Idempotency Payments API example and split out JMESPath Function section from Validation utility since also applicable to Idempotency utility. This split seems logical and updated references in both Validation and Idempotnecy documentation to suit.

No changes for Feature Flags documentation required (JMESPath is referenced but not built-in functions specifically)

Added warning around JSON API Idempotency in the Payments example explicitly calling ou the usage of event_key_jmespath which is essential for correct Idempotency for JSON API structures.

Checklist

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.


View rendered docs/utilities/idempotency.md
View rendered docs/utilities/jmespath_functions.md
View rendered docs/utilities/validation.md

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Aug 28, 2021
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 28, 2021
@codecov-commenter
Copy link

codecov-commenter commented Aug 28, 2021

Codecov Report

Merging #661 (da73191) into develop (c8cf3ba) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #661   +/-   ##
========================================
  Coverage    99.97%   99.97%           
========================================
  Files          116      116           
  Lines         4846     4866   +20     
  Branches       265      267    +2     
========================================
+ Hits          4845     4865   +20     
  Partials         1        1           
Impacted Files Coverage Δ
aws_lambda_powertools/event_handler/api_gateway.py 100.00% <0.00%> (ø)
...mbda_powertools/utilities/validation/exceptions.py 100.00% <0.00%> (ø)
...ools/utilities/idempotency/persistence/dynamodb.py 100.00% <0.00%> (ø)
...ities/data_classes/api_gateway_authorizer_event.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8cf3ba...da73191. Read the comment docs.

@walmsles walmsles changed the title Docs/idempotency fix: Idempotent documentation examples could be misleading #657 Aug 28, 2021
@walmsles walmsles changed the title fix: Idempotent documentation examples could be misleading #657 fix: Idempotent documentation examples could be misleading Aug 28, 2021
@heitorlessa heitorlessa requested a review from to-mc August 31, 2021 17:40
Copy link
Contributor

@to-mc to-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @walmsles! A few suggested changes inline - definitely a good improvement for the docs 😊

docs/utilities/idempotency.md Outdated Show resolved Hide resolved
docs/utilities/idempotency.md Outdated Show resolved Hide resolved
docs/utilities/idempotency.md Outdated Show resolved Hide resolved
docs/utilities/jmespath_functions.md Outdated Show resolved Hide resolved
Apply suggestions from code review

Co-authored-by: Tom McCarthy <[email protected]>
@walmsles
Copy link
Contributor Author

@cakepietoast - accepted review suggestions - documentation IS hard, thanks for the improvements 👍

Copy link
Contributor

@to-mc to-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@heitorlessa heitorlessa changed the title fix: Idempotent documentation examples could be misleading docs(idempotency): fix misleading idempotent examples Sep 28, 2021
@heitorlessa
Copy link
Contributor

Absolutely loved it!! Now that we're exposing JMESPath Functions in its own doc, I think it's fair we expose it for everyone to use with jmespath whether they use idempotency or validation.

Since jmespath dependency is brought along, they will be able to extract things from JSON more easily too.

@heitorlessa heitorlessa merged commit 204eb98 into aws-powertools:develop Sep 28, 2021
@walmsles
Copy link
Contributor Author

@heitorlessa also within Feature Toggles - although that doc references JMESPath site directly I noticed so within the scope of this change left that alone. Maybe want to re-align those docs too?

heitorlessa added a commit to heitorlessa/aws-lambda-powertools-python that referenced this pull request Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants