fix(dynamoDB): make TableV2 taggable by 5d · Pull Request #31867 · aws/aws-cdk

5d · 2024-10-23T20:27:10Z

Issue # (if applicable)

Reason for this change

The AWS::DynamoDB::GlobalTable resource does not natively support tags. However, at the L2 level (TableV2), the tags option is available when creating this resource, which may give customers the impression that it is taggable. When customers apply tags using the aspect at the TableV2 level, it’s likely they intend to tag all replicas, as each replica has its own tags property as well. This behavior also aligns with the related construct documentation.

Description of changes

Enable tagging for TableV2
If a tag key is defined in the replica tags properties, it will take precedence over the TagAspect for that key, as it is more specific.
Keep the tags property behavior in TableV2
- Update the description doc on it to reflect that it only applies to the default replica table

Description of how you validated changes

added new unit test cases
deployed related integration test cases

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

GavinZZ

IMO I think this is an issue with the spec2cdk generation and something we should fix in the L1 generation instead of L2 side.

packages/aws-cdk-lib/aws-dynamodb/lib/table-v2.ts

packages/aws-cdk-lib/aws-dynamodb/test/table-v2.test.ts

.../aws-dynamodb/test/integ.dynamodb-v2.ondemand.js.snapshot/aws-cdk-global-table.template.json

GavinZZ · 2024-10-24T21:58:28Z

Thanks for addressing the other comments. I'm happy to approve once we check with the service team and update the documentation.

5d · 2024-10-24T22:05:32Z

Thanks for addressing the other comments. I'm happy to approve once we check with the service team and update the documentation.

Thanks for reviewing this! @GavinZZ, I'll follow up with the product team.

Pull request has been modified.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

packages/aws-cdk-lib/aws-dynamodb/lib/table-v2.ts

packages/aws-cdk-lib/aws-dynamodb/test/table-v2.test.ts

moelasmar

In general looks good to me, I just left some minor comments.

Pull request has been modified.

mergify · 2024-11-02T02:43:23Z

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).

mergify · 2024-11-02T02:47:14Z

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-cdk-automation · 2024-11-02T03:39:22Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 4506d29
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-11-02T03:39:51Z

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).

github-actions · 2024-11-02T03:40:07Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

5d added 2 commits October 23, 2024 12:43

fix(dynamoDB): add tags to TableV2

fd5dc19

update test cases

ee7db95

github-actions bot added the p2 label Oct 23, 2024

aws-cdk-automation requested a review from a team October 23, 2024 20:27

github-actions bot added the repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK label Oct 23, 2024

5d added 2 commits October 23, 2024 13:57

fix broken unit tests

0b1e04c

Merge remote-tracking branch 'origin/main' into 5d/fix-issue-30631

5c4626b

5d changed the title ~~fix(dynamoDB): add tags to TableV2~~ fix(dynamoDB): make TableV2 taggable Oct 23, 2024

5d marked this pull request as ready for review October 23, 2024 21:54

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 23, 2024

github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort labels Oct 23, 2024

GavinZZ previously requested changes Oct 24, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 24, 2024

github-actions bot mentioned this pull request Oct 28, 2024

Weekly PR metrics report #31915

Closed

Merge remote-tracking branch 'origin/main' into 5d/fix-issue-30631

17cbe63

5d force-pushed the 5d/fix-issue-30631 branch from 15d8a27 to 4e2481e Compare October 29, 2024 19:58

aws-cdk-automation previously requested changes Oct 29, 2024

View reviewed changes

keep the tags behavior on TableV2

4adfb4d

5d force-pushed the 5d/fix-issue-30631 branch from 4e2481e to 4adfb4d Compare October 29, 2024 21:05

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 29, 2024

5d requested a review from GavinZZ October 29, 2024 21:48

moelasmar reviewed Oct 31, 2024

View reviewed changes

packages/aws-cdk-lib/aws-dynamodb/lib/table-v2.ts Show resolved Hide resolved

moelasmar reviewed Oct 31, 2024

View reviewed changes

packages/aws-cdk-lib/aws-dynamodb/lib/table-v2.ts Show resolved Hide resolved

moelasmar reviewed Oct 31, 2024

View reviewed changes

packages/aws-cdk-lib/aws-dynamodb/test/table-v2.test.ts Show resolved Hide resolved

moelasmar previously requested changes Oct 31, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 31, 2024

5d added 2 commits October 31, 2024 10:06

Merge remote-tracking branch 'origin/main' into 5d/fix-issue-30631

d3c15ae

add more unit and integ test cases

4b1d674

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 31, 2024

github-actions bot mentioned this pull request Nov 1, 2024

Monthly PR metrics report #31972

Closed

moelasmar approved these changes Nov 2, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 2, 2024

Merge branch 'main' into 5d/fix-issue-30631

4506d29

mergify bot merged commit 796c6d1 into main Nov 2, 2024

mergify bot deleted the 5d/fix-issue-30631 branch November 2, 2024 03:39

github-actions bot locked as resolved and limited conversation to collaborators Nov 2, 2024

Conversation

5d commented Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Uh oh!

GavinZZ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GavinZZ commented Oct 24, 2024

Uh oh!

5d commented Oct 24, 2024

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

moelasmar left a comment

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Nov 2, 2024

Uh oh!

mergify bot commented Nov 2, 2024

Uh oh!

aws-cdk-automation commented Nov 2, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Nov 2, 2024

Uh oh!

github-actions bot commented Nov 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

5d commented Oct 23, 2024 •

edited

Loading