feat(dynamodb): Implement importing existing dynamodb table. #5280

zxkane · 2019-12-03T08:47:11Z

Closes #3895.

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

mergify · 2019-12-03T08:47:50Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

aws-cdk-automation · 2019-12-03T08:54:10Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2019-12-03T10:05:08Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2019-12-03T10:42:19Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

zxkane · 2019-12-14T15:03:32Z

@skinny85 any feedback?

mergify · 2019-12-16T12:33:21Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

mergify · 2019-12-16T13:00:47Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

mergify · 2019-12-16T13:53:06Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

mergify · 2019-12-16T14:38:47Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

mergify · 2019-12-16T14:52:20Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

mergify · 2019-12-16T15:17:25Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

skinny85

Thanks for the contribution @zxkane ! I have some comments before I feel comfortable merging this in.

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

packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts

Pull request has been modified.

aws-cdk-automation · 2019-12-18T15:26:09Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2019-12-18T16:08:51Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2019-12-18T16:11:09Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

skinny85

We're almost there!

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

skinny85 · 2019-12-23T22:09:34Z

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

+    if (attrs.tableName) {
+      tableName = attrs.tableName;
+      if (!attrs.tableArn) {
+        tableArn = stack.formatArn({


This should be:

tableArn = stack.formatArn({ service: 'dynamodb', resource: 'table', resourceName: attrs.tableName, });

Per the doc https://docs.aws.amazon.com/IAM/latest/UserGuide/list_amazondynamodb.html#amazondynamodb-resources-for-iam-policies, are region and account are mandatory for a valid DynamoDB ARN?

Right. It's just that if you don't pass anything for account and region, defaults will be used (see here:

aws-cdk/packages/@aws-cdk/core/lib/arn.ts

Lines 81 to 82 in 2df88f3

const region = components.region !== undefined ? components.region : stack.region;

const account = components.account !== undefined ? components.account : stack.account;

).

So no reason to pass these explicitly.

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

aws-cdk-automation · 2019-12-24T08:40:29Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

Closes aws#3895.

aws-cdk-automation · 2019-12-24T14:46:01Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

skinny85

Sweet, thanks for the awesome contribution @zxkane !

mergify · 2019-12-26T20:04:59Z

Thank you for contributing! Your pull request is now being automatically merged.

aws-cdk-automation · 2019-12-26T20:34:38Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2019-12-26T20:35:03Z

Thank you for contributing! Your pull request is now being automatically merged.

zxkane requested a review from skinny85 as a code owner December 3, 2019 08:47

zxkane force-pushed the dynamodb-import branch from 010e66d to 3b3fa61 Compare December 3, 2019 09:58

zxkane force-pushed the dynamodb-import branch from 3b3fa61 to 4164f32 Compare December 3, 2019 10:17

SomayaB assigned skinny85 Dec 3, 2019

skinny85 previously requested changes Dec 16, 2019

View reviewed changes

zxkane force-pushed the dynamodb-import branch from 4164f32 to aac9b18 Compare December 18, 2019 14:53

zxkane force-pushed the dynamodb-import branch 2 times, most recently from 90b82f3 to 4786670 Compare December 18, 2019 15:43

skinny85 reviewed Dec 23, 2019

View reviewed changes

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

zxkane force-pushed the dynamodb-import branch from 4786670 to d59d646 Compare December 24, 2019 08:06

feat(dynamodb): Implement importing existing dynamodb table.

05448d5

Closes aws#3895.

zxkane force-pushed the dynamodb-import branch from d59d646 to 05448d5 Compare December 24, 2019 14:19

skinny85 approved these changes Dec 26, 2019

View reviewed changes

Merge branch 'master' into dynamodb-import

da871d1

mergify bot merged commit 8d9b58b into aws:master Dec 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dynamodb): Implement importing existing dynamodb table. #5280

feat(dynamodb): Implement importing existing dynamodb table. #5280

zxkane commented Dec 3, 2019

mergify bot commented Dec 3, 2019

aws-cdk-automation commented Dec 3, 2019

aws-cdk-automation commented Dec 3, 2019

aws-cdk-automation commented Dec 3, 2019

zxkane commented Dec 14, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

skinny85 left a comment

aws-cdk-automation commented Dec 18, 2019

aws-cdk-automation commented Dec 18, 2019

aws-cdk-automation commented Dec 18, 2019

skinny85 left a comment

skinny85 Dec 23, 2019

zxkane Dec 24, 2019

skinny85 Dec 26, 2019

aws-cdk-automation commented Dec 24, 2019

aws-cdk-automation commented Dec 24, 2019

skinny85 left a comment

mergify bot commented Dec 26, 2019

aws-cdk-automation commented Dec 26, 2019

mergify bot commented Dec 26, 2019

	const region = components.region !== undefined ? components.region : stack.region;
	const account = components.account !== undefined ? components.account : stack.account;

feat(dynamodb): Implement importing existing dynamodb table. #5280

feat(dynamodb): Implement importing existing dynamodb table. #5280

Conversation

zxkane commented Dec 3, 2019

mergify bot commented Dec 3, 2019

aws-cdk-automation commented Dec 3, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Dec 3, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Dec 3, 2019

AWS CodeBuild CI Report

zxkane commented Dec 14, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

mergify bot commented Dec 16, 2019

skinny85 left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Dec 18, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Dec 18, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Dec 18, 2019

AWS CodeBuild CI Report

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 Dec 23, 2019

Choose a reason for hiding this comment

zxkane Dec 24, 2019

Choose a reason for hiding this comment

skinny85 Dec 26, 2019

Choose a reason for hiding this comment

aws-cdk-automation commented Dec 24, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Dec 24, 2019

AWS CodeBuild CI Report

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 26, 2019

aws-cdk-automation commented Dec 26, 2019

AWS CodeBuild CI Report

mergify bot commented Dec 26, 2019