feat(redshift): multi AZ cluster #29976

badmintoncryer · 2024-04-26T13:41:39Z

Issue # (if applicable)

Closes #.

Reason for this change

AWS CDK cannot configure Redshift multi-AZ cluster.

Description of changes

Add multiAz to clusterProps.

Description of how you validated changes

I've added both unit and integ tests.

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

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.

go-to-k

Thanks for this PR. I made just some minor comments.

packages/@aws-cdk/aws-redshift-alpha/README.md

packages/@aws-cdk/aws-redshift-alpha/lib/cluster.ts

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

go-to-k

This comment (And it would be good to include an explanation for the need of the multiAz property.) has not been reflected yet, so it may be in the process of being corrected, but I wanted to comment first on what I noticed.

packages/@aws-cdk/aws-redshift-alpha/lib/cluster.ts

packages/@aws-cdk/aws-redshift-alpha/test/cluster.test.ts

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

badmintoncryer · 2024-05-02T13:40:01Z

@go-to-k Thank you for your review!! I'm sorry that I've pushed temporary commits. I've addressed all your comments.

go-to-k · 2024-05-02T13:55:41Z

Oh... failed...

go-to-k · 2024-05-02T14:10:21Z

Oh... failed...

Succeeded! I'll check them.

go-to-k

LGTM. Thanks for the changes!

badmintoncryer · 2024-05-02T14:25:37Z

Thank you always!

aws-cdk-automation · 2024-05-05T20:42:27Z

AWS CodeBuild CI Report

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

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

shikha372 · 2024-05-10T21:09:23Z

packages/@aws-cdk/aws-redshift-alpha/lib/cluster.ts

@@ -558,6 +565,17 @@ export class Cluster extends ClusterBase {
      );
    }

+    const nodeType = props.nodeType || NodeType.DC2_LARGE;


do we need to specify it here if its already set as the default for this prop?

This specification is not new.

https://github.com/aws/aws-cdk/pull/29976/files#diff-8666885f385d3c87fb98f835d762b86e7a98bfd217bd69c9a210041ab07585d6L577

And cloudformation requires this prop.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-redshift-cluster.html#cfn-redshift-cluster-nodetype

If we change this implementation, it will cause a breaking change. Therefore, I think it is better to keep it as is.

aws-cdk-automation requested a review from a team April 26, 2024 13:41

github-actions bot added p2 star-contributor [Pilot] contributed between 25-49 PRs to the CDK labels Apr 26, 2024

aws-cdk-automation previously requested changes Apr 26, 2024

View reviewed changes

badmintoncryer force-pushed the multiAzRedshiftCluster branch from f60e559 to 6060084 Compare April 28, 2024 15:25

badmintoncryer marked this pull request as ready for review April 28, 2024 15:28

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

badmintoncryer marked this pull request as draft April 29, 2024 01:58

badmintoncryer changed the title ~~feat(redshift-alpha): multi AZ cluster~~ feat(redshift): multi AZ cluster Apr 29, 2024

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

badmintoncryer marked this pull request as ready for review April 29, 2024 03:57

github-actions bot mentioned this pull request Apr 29, 2024

Weekly PR metrics report #29994

Closed

badmintoncryer force-pushed the multiAzRedshiftCluster branch from 262fb29 to 91d89bc Compare April 29, 2024 12:06

badmintoncryer marked this pull request as draft April 29, 2024 12:06

badmintoncryer requested a deployment to test-pipeline April 29, 2024 12:06 — with GitHub Actions Waiting

badmintoncryer added 9 commits April 29, 2024 21:07

feat: multi az

8278e1c

test: add integ test

97176b1

fix: remove unnecessary integ test

d7ba12e

chore: remove line feed

804b9ff

docs: update readme

9826834

fix: stack account

7720ac9

fix: integ test

6d7629b

chore: refactor unit test

1afd15c

test: update integ test

b1de6a6

badmintoncryer force-pushed the multiAzRedshiftCluster branch from 91d89bc to b1de6a6 Compare April 29, 2024 12:07

badmintoncryer marked this pull request as ready for review April 29, 2024 12:08

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

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

Monthly PR metrics report #30026

Closed

go-to-k suggested changes May 1, 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 May 1, 2024

badmintoncryer and others added 2 commits May 2, 2024 16:26

Update packages/@aws-cdk/aws-redshift-alpha/lib/cluster.ts

7c78475

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

fix: adrressing review comment

75bb92b

go-to-k suggested changes May 2, 2024

View reviewed changes

badmintoncryer and others added 4 commits May 2, 2024 20:09

Update packages/@aws-cdk/aws-redshift-alpha/lib/cluster.ts

ba254a9

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

Update packages/@aws-cdk/aws-redshift-alpha/lib/cluster.ts

61e786a

Co-authored-by: k.goto <24818752+go-to-k@users.noreply.github.com>

docs: update readme

13574d9

fix: unit test

de29024

Merge branch 'main' into multiAzRedshiftCluster

d028b2f

go-to-k approved these changes May 2, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 2, 2024

shikha372 self-assigned this May 3, 2024

Merge branch 'main' into multiAzRedshiftCluster

0cdaa0b

shikha372 reviewed May 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(redshift): multi AZ cluster #29976

feat(redshift): multi AZ cluster #29976

badmintoncryer commented Apr 26, 2024 •

edited

aws-cdk-automation left a comment

go-to-k left a comment

go-to-k left a comment

badmintoncryer commented May 2, 2024

go-to-k commented May 2, 2024

go-to-k commented May 2, 2024

go-to-k left a comment

badmintoncryer commented May 2, 2024

aws-cdk-automation commented May 5, 2024

shikha372 May 10, 2024 •

edited

badmintoncryer May 11, 2024 •

edited

feat(redshift): multi AZ cluster #29976

Are you sure you want to change the base?

feat(redshift): multi AZ cluster #29976

Conversation

badmintoncryer commented Apr 26, 2024 • edited

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

go-to-k left a comment

Choose a reason for hiding this comment

go-to-k left a comment

Choose a reason for hiding this comment

badmintoncryer commented May 2, 2024

go-to-k commented May 2, 2024

go-to-k commented May 2, 2024

go-to-k left a comment

Choose a reason for hiding this comment

badmintoncryer commented May 2, 2024

aws-cdk-automation commented May 5, 2024

AWS CodeBuild CI Report

shikha372 May 10, 2024 • edited

Choose a reason for hiding this comment

badmintoncryer May 11, 2024 • edited

Choose a reason for hiding this comment

badmintoncryer commented Apr 26, 2024 •

edited

shikha372 May 10, 2024 •

edited

badmintoncryer May 11, 2024 •

edited