-
Notifications
You must be signed in to change notification settings - Fork 402
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(event-sources): cache parsed json in data class #909
Conversation
A micro optimization to cache the parsed json within the event source data class
Codecov Report
@@ Coverage Diff @@
## develop #909 +/- ##
===========================================
+ Coverage 99.90% 99.92% +0.01%
===========================================
Files 118 118
Lines 5246 5255 +9
Branches 596 600 +4
===========================================
+ Hits 5241 5251 +10
Misses 1 1
+ Partials 4 3 -1
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.
I'm fine with this solution - better than actual caching as UX and memory would've been tricky.
As we're gonna do it, let's revisit all other fields that do any sort of deserialisation -- Kinesis is one that does it heavy (json + b64) etc
@heitorlessa ok. I will update others like this too. |
@heitorlessa - ok i have added various singleton caching options. |
This looks great! Looking forward to applying it. |
I was gonna suggest a decorator but given the amount of fields we're fine. If we get a feature request to optimize other fields that require expensive operation like decoding & unzipping CloudWatch Logs record, or decoding and JSON parsing Kinesis records, then we should refactor this to allow for growth. Future note: If more fields are required, we should convert this into a reusable one-item |
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 one question on the usefulness of tests; happy to merge otherwise
@heitorlessa - was thinking of a |
Like this but we don't need the threading safety I believe, as it'll compute once and more lightweight and faster than the LRU cache - once we deprecate older Python versions we can use it natively. We would need to test whether that work for our data types tho, but should be easily adjustable as the logic is simple |
Merging as is - I'll look into the hash dunder and decorator when Mypy is fully supported across the codebase |
…tools-python into complex * 'develop' of https://github.com/awslabs/aws-lambda-powertools-python: (24 commits) docs: consistency around admonitions and snippets (aws-powertools#919) chore(deps-dev): bump mypy from 0.920 to 0.930 (aws-powertools#925) fix(event-sources): handle dynamodb null type as none, not bool (aws-powertools#929) fix(apigateway): support @app.not_found() syntax & housekeeping (aws-powertools#926) docs: Added GraphQL Sample API to Examples section of README.md (aws-powertools#930) feat(idempotency): support dataclasses & pydantic models payloads (aws-powertools#908) feat(tracer): ignore tracing for certain hostname(s) or url(s) (aws-powertools#910) feat(event-sources): cache parsed json in data class (aws-powertools#909) fix(warning): future distutils deprecation (aws-powertools#921) docs(batch): remove leftover from legacy docs(layer): bump Lambda Layer to version 6 chore: bump to 1.23.0 docs(apigateway): add new not_found feature (aws-powertools#915) docs: external reference to cloudformation custom resource helper (aws-powertools#914) feat(logger): allow handler with custom kwargs signature (aws-powertools#913) chore: minor housekeeping before release (aws-powertools#912) chore(deps-dev): bump mypy from 0.910 to 0.920 (aws-powertools#903) feat(batch): new BatchProcessor for SQS, DynamoDB, Kinesis (aws-powertools#886) fix(parser): overload parse when using envelope (aws-powertools#885) fix(parser): kinesis sequence number is str, not int (aws-powertools#907) ...
…tools-python into feature/905-datetime * 'develop' of https://github.com/awslabs/aws-lambda-powertools-python: feat(feature_flags): support beyond boolean values (JSON values) (aws-powertools#804) docs: consistency around admonitions and snippets (aws-powertools#919) chore(deps-dev): bump mypy from 0.920 to 0.930 (aws-powertools#925) fix(event-sources): handle dynamodb null type as none, not bool (aws-powertools#929) fix(apigateway): support @app.not_found() syntax & housekeeping (aws-powertools#926) docs: Added GraphQL Sample API to Examples section of README.md (aws-powertools#930) feat(idempotency): support dataclasses & pydantic models payloads (aws-powertools#908) feat(tracer): ignore tracing for certain hostname(s) or url(s) (aws-powertools#910) feat(event-sources): cache parsed json in data class (aws-powertools#909) fix(warning): future distutils deprecation (aws-powertools#921)
Issue #, if available:
Description of changes:
A micro optimization to cache the parsed json within the event source data class
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.