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
Enhance parity for DynamoDB to Kinesis stream integration #10143
Conversation
@@ -238,55 +238,6 @@ def test_list_tags_of_resource(self, aws_client): | |||
|
|||
aws_client.dynamodb.delete_table(TableName=table_name) | |||
|
|||
@markers.aws.only_localstack |
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.
Moving this test to a new test_dynamodb_streams.py
file (only refactoring, no functional changes)..
LocalStack Community integration with Pro 2 files 2 suites 1h 21m 21s ⏱️ Results for commit 66b875a. ♻️ This comment has been updated with latest results. |
I can add a comment related to the comment: for The comment was the only change in Refactoring |
@bentsku Kudos for fixing the tests and pushing the PR over the line! 🚀 I cannot approve my own PR, but LGTM! :) |
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.
I'll also approve my own work 😄 the -ext pipeline is also all green for this PR!
Also, thanks @whummer for updating the PR description with my changes as well. Nice to see boto made it much cleaner about the dynamize
util function, we also have some unit tests for it and they are still passing!
Co-authored-by: Benjamin Simon <benjh.simon@gmail.com>
Motivation
Following a customer request, it seems that our integration between DynamoDB tables and Kinesis streams is not fully working for certain DDB operations, in particular when using the
transact_write_items
operation. (Apparently it worked for older versions prior to 3.0, but I haven't confirmed).🚧 At this point, the PR only adds a snapshot test to illustrate the behavior - proper logic still needs to be implemented. (may need some help from service owners here, if possible 🙌 )
Wondering if this TODO comment could be related (as I know @bentsku has been working on some pretty significant performance enhancements recently 🚀 ):
localstack/localstack/services/dynamodb/provider.py
Line 1112 in 45d39ed
Update: After discussing with @bentsku , the TODO above is not related - the fix was essentially related to converting a table ARN to the table name.
Changes
has_streams_enabled(..)
to extract the table name from incoming table ARNrecord
s, for better parityde_dynamize_record
util function and refactordynamize_value
to use botoTypeSerializer
utilstest_enable_kinesis_streaming_destination
snapshot test to cover the functionality