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

feat(idempotent): Add support for jmespath_options #302

Merged
merged 6 commits into from Mar 4, 2021

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Mar 3, 2021

Issue #, if available:

Description of changes:

Like for the validator, idempotent utility should allow for extracting an idempotent key using custom functions

Example usage:

We have an api gateway proxy event:

Where event is:

{
   "body": "{\"id\": \"c299229\"}"
}

And c299229 is the idempotent key we want to use:

@idempotent(DynamoDBPersistenceLayer(table_name="foo", event_key_jmespath="powertools_json(body).id"))
def handler(event, context):
    pass

Checklist

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

Like for the validator, idempotent utility should allow for extracting an idempotent key using custom functions
@michaelbrewer michaelbrewer changed the title feat(idempotent): Add support for jmespath_options feat(idempotent): Add support for jmespath_options (PROPOSAL) Mar 3, 2021
@michaelbrewer michaelbrewer marked this pull request as draft March 3, 2021 04:13
@codecov-io
Copy link

codecov-io commented Mar 3, 2021

Codecov Report

Merging #302 (7df82ff) into develop (d7b4afe) will decrease coverage by 0.03%.
The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #302      +/-   ##
===========================================
- Coverage    99.71%   99.68%   -0.04%     
===========================================
  Files           87       87              
  Lines         3151     3155       +4     
  Branches       150      151       +1     
===========================================
+ Hits          3142     3145       +3     
  Misses           5        5              
- Partials         4        5       +1     
Impacted Files Coverage Δ
...wertools/utilities/idempotency/persistence/base.py 98.43% <80.00%> (-0.76%) ⬇️

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 d7b4afe...7df82ff. Read the comment docs.

@michaelbrewer michaelbrewer marked this pull request as ready for review March 3, 2021 04:45
@michaelbrewer
Copy link
Contributor Author

@cakepietoast @heitorlessa i can update the docs, if this feature makes sense?

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

LGTM after docs mention this -- In all fairness, simply copy and replace what's available in Validations doc, and we can refactor that to make it reusable later.

https://awslabs.github.io/aws-lambda-powertools-python/utilities/validation/#built-in-jmespath-functions

@michaelbrewer michaelbrewer changed the title feat(idempotent): Add support for jmespath_options (PROPOSAL) feat(idempotent): Add support for jmespath_options Mar 3, 2021
Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

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

Last change and I'll merge it :)

Here's the inner cache of jmespath parser - used both by compile and search: https://github.com/jmespath/jmespath.py/blob/599b0f7ea650a60f7512233301e70ff8151de0f0/jmespath/parser.py#L84

Here's the subtle difference between compile and search: https://github.com/jmespath/jmespath.py/blob/599b0f7ea650a60f7512233301e70ff8151de0f0/jmespath/__init__.py#L23

Contrary to fastjsonschema that doesn't cache, JMESPath does on both occasions, and keep up to 128 expressions.

@heitorlessa
Copy link
Contributor

Thanks Michael, again! I'm merging as-is, and create a reminder to come back to simplify the internals once I have a better understanding of Tom's implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants