-
Notifications
You must be signed in to change notification settings - Fork 464
fix(event-sources): handle dynamodb null type as none, not bool #929
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
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #929 +/- ##
===========================================
- Coverage 99.88% 99.88% -0.01%
===========================================
Files 118 118
Lines 5272 5271 -1
Branches 605 605
===========================================
- Hits 5266 5265 -1
Misses 2 2
Partials 4 4
Continue to review full report at Codecov.
|
|
|
||
|
|
||
| def test_dynamo_attribute_value_null_value_invalid(): | ||
| example_attribute_value = {"NULL": False} |
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.
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.
@michaelbrewer - We are deserializing, {"NULL": False} should never happen (so should we raise a ValueError or just ignore that?)
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.
@heitorlessa - i can remove that test, i just wanted it to be clear that we don't valid
@property
def null_value(self) -> None:
"""An attribute of type Null.
Example:
>>> {"NULL": True}
"""
return None
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.
which is the same as:
def _deserialize_null(self, value):
return None
heitorlessa
left a comment
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 on whether NULL will ever be False, boto tests always caters for True
Type table only considers True so perhaps DynamoDB doesn't compute that: https://github.com/boto/boto3/blob/cf62d26cb0f28357c935d541ed3f0dae196cd26d/boto3/dynamodb/types.py#L85
Because boto does not allow you to call |
…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:
Return for
Nonefor null types:Example:
Below
null_valueandget_valuewould have returnedTrue, and now they return None.Checklist
Breaking change checklist
Before
null_valuewas returning a boolean,TruewhenNULLvalue was set totrueandFalsewhenNULLvalue was not set (or wasFalse)By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.