-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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(appsync): support for read consistency during DynamoDB reads #20793
Conversation
b35dc51
to
1d04a0a
Compare
Overall looks fine, but I'd rather see a test that shows that passing w/r/t the README, as a nit: It might be nice to have a link to the documentation ( https://docs.aws.amazon.com/appsync/latest/devguide/resolver-mapping-template-reference-dynamodb.html ) on how resolver mappings work and how you're passing them under the hood. |
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'd like to see at least in integ snapshots a test with none/true/false resolvers just for consistency across the full suite.
Thanks for your comment. |
chore: add unit test for dynamoDbScanTable
…s-cdk into read-consistency-v3
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 am not sure if we should remove the comments in the README. I am fine with either.
Co-authored-by: Pahud Hsieh <[email protected]>
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.
LGTM.
This covers every base and does a fantastic job of making sure it covers all the corner cases. 👍
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…s#20793) ---- Motivation: DynamoDB uses eventually consistent reads unless you specify otherwise. Now the users cannot specify consistent read during DynamoDB reads. This commit allows users to set consistent read while creating resolvers. Use case: To control the amount of replicas contacted during DynamoDB reads. most salient design aspects: Added an boolean parameter to the dynamoDbGetItem, dynamoDbScanTable and dynamoDbQuery methods in mapping-template.ts. Changed relative tests. closes aws#5980 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features I ran yarn tests. They include integration test snapshots for DynamoDB reads. Do I need to write a new integration test? * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…s#20793) ---- Motivation: DynamoDB uses eventually consistent reads unless you specify otherwise. Now the users cannot specify consistent read during DynamoDB reads. This commit allows users to set consistent read while creating resolvers. Use case: To control the amount of replicas contacted during DynamoDB reads. most salient design aspects: Added an boolean parameter to the dynamoDbGetItem, dynamoDbScanTable and dynamoDbQuery methods in mapping-template.ts. Changed relative tests. closes aws#5980 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features I ran yarn tests. They include integration test snapshots for DynamoDB reads. Do I need to write a new integration test? * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Motivation: DynamoDB uses eventually consistent reads unless you specify otherwise. Now the users cannot specify consistent read during DynamoDB reads. This commit allows users to set consistent read while creating resolvers.
Use case: To control the amount of replicas contacted during DynamoDB reads.
most salient design aspects:
Added an boolean parameter to the dynamoDbGetItem, dynamoDbScanTable and dynamoDbQuery methods in mapping-template.ts. Changed relative tests.
closes #5980
All Submissions:
Adding new Unconventional Dependencies:
New Features
I ran yarn tests. They include integration test snapshots for DynamoDB reads. Do I need to write a new integration test?
yarn integ
to deploy the infrastructure and generate the snapshot (i.e.yarn integ
without--dry-run
)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license