-
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(aws-dynamodb-global): Adding package to support DynamoDB Global Tables #2251
feat(aws-dynamodb-global): Adding package to support DynamoDB Global Tables #2251
Conversation
Pull master
Update to 0.28
Update from master
fix(decdk): set the timeout in the schema tests to 10 seconds. (#2250)
fix(codebuild): add validation for Source when the badge property is …
…Poptart/aws-cdk into KingOfPoptart/aws-dynamodb-global
So for clarity - I completely screwed up my previous PR (rebasing nightmare from waiting 20+ days between starting and getting ready to merge), so I'm creating this one to replace it. Old PR can be found here - #2082 |
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.
A few more comments...
* Permits an IAM Principal to create a global dynamodb table. | ||
* @param principal The principal (no-op if undefined) | ||
*/ | ||
public static grantCreateGlobalTableLambda(principal?: iam.IPrincipal): void { |
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.
Why does this need to be public? Seems like it's basically a helper used only in this file.
|
||
this.lambdaGlobalDynamodbMaker = new LambdaGlobalDynamoDBMaker(scope, id + "-CustomResource", props); | ||
for (const table of this._regionalTables) { | ||
this.lambdaGlobalDynamodbMaker.customResource.node.addDependency(table); |
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 believe (not 100% sure) that if you just do this.lambdaGlobalDynamodbMaker.node.addDependency(table)
it will automatically add a dependency on all underlying resources
* A stack that will make a Lambda that will launch a lambda to glue | ||
* together all the DynamoDB tables into a global table | ||
*/ | ||
export class LambdaGlobalDynamoDBMaker extends cdk.Stack { |
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.
Sorry for nitpicking, I am a bit sensitive to naming. Can you rename this to GlobalTableCoordinator
(it's an internal class, I know...).
lambdaProvider: this.lambdaFunction, | ||
properties: { | ||
regions: props.regions, | ||
resourceType: "Custom::MakeGlobalDynamoDB", |
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.
Rename to Custom::DynamoGlobalTableCoordinator
"sphinx": {} | ||
} | ||
}, | ||
"cdk-build": { |
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.
Remove this from here... It's only for the aws-dynamodb package
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.
Looks awesome. Thanks for all the hard work!
@KingOfPoptart do you mind taking a few minutes finalizing the PR title and message so we can use them for the merge commit? |
@eladb updated! Let me know if that's sufficient. |
Adding a new package to support AWS DynamoDB Global Tables. The new package is built on top the existing aws-dynamodb package.
The construct creates multiple stacks in the specified regions, each with a DynamoDB Table (using the aws-dynamodb package). A custom resource is then deployed and uses a lambda to tie the tables together into a multi-region, multi-master database.
Pull Request Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.