fix(rds): clusterScailabilityType is spelled wrong and should be clusterScalabilityType #32825

iankhou · 2025-01-09T23:25:23Z

Issue #32415

Closes #32415 .

Reason for this change

Misspelling of the ClusterScalabilityType type, enum, and prop.

Description of changes

Deprecated misspellings of ClusterScailabilityType/clusterScailabilityType and aliased the misspelling for backwards compatibility. The misspelled name will be removed in the next MV release.

Describe any new or updated permissions being added

No permissions changes.

Description of how you validated changes

Added unit tests for the new spelling (ClusterScalabilityType/clusterScalabilityType), and kept unit tests that tested the misspelling.

# in packages/aws-cdk-lib
yarn test aws-rds

Ran the relevant integration test.

# in packages/@aws-cdk-testing/framework-integ
yarn integ test/aws-rds/test/integ.cluster-limitless.js

Ran Rosetta to verify README changes.

./scripts/run-rosetta.sh

No complications from README changes in these commits.

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

codecov · 2025-01-09T23:45:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.52%. Comparing base (f63ec29) to head (facddcd).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32825   +/-   ##
=======================================
  Coverage   81.52%   81.52%           
=======================================
  Files         222      222           
  Lines       13717    13717           
  Branches     2417     2417           
=======================================
  Hits        11183    11183           
  Misses       2254     2254           
  Partials      280      280

Flag	Coverage Δ
suite.unit	`81.52% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`80.98% <ø> (ø)`
packages/aws-cdk-lib/core	`82.09% <ø> (ø)`

aws-cdk-automation · 2025-01-10T02:17:53Z

AWS CodeBuild CI Report

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

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

rix0rrr · 2025-01-10T14:41:45Z

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

-    if (props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS) {
+    if (props.clusterScalabilityType === ClusterScalabilityType.LIMITLESS || props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS) {


Also needs a check to ensure that you can't set both the old and new property at the same time. And that should be mentioned in the docstring as well.

Thanks, will make this change.

rix0rrr · 2025-01-10T14:44:30Z

packages/aws-cdk-lib/aws-rds/test/cluster.test.ts

@@ -419,6 +440,240 @@ describe('cluster new api', () => {
      });
    });

+    describe('limitless database', () => {


Lotsa new tests for a simple feature? Copy/paste or GenAI?

I'd rather turn existing tests into data-driven tests for both ways of passing props.

test.each([ { clusterScailabilityType: ClusterScailabilityType.LIMITLESS }, { clusterScalabilityType: ClusterScalabilityType.LIMITLESS }, ])('my test with input %p, (partialProps) => { // ... });

The intention was to duplicate the existing tests to take either the misspelled or the corrected name (and to remove the tests for the misspelling after the MV bump). Will look into refactoring them so that we don't have so much extra code.

iankhou · 2025-01-10T16:28:56Z

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

@@ -701,6 +730,10 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
  constructor(scope: Construct, id: string, props: DatabaseClusterBaseProps) {
    super(scope, id);

+    if (props.clusterScalabilityType !== undefined && props.clusterScailabilityType !== undefined) {
+      throw new Error('You cannot specify both clusterScalabilityType and clusterScailabilityType (deprecated). Use clusterScalabilityType.');


@rix0rrr here I disallow both options to be set.

iankhou · 2025-01-10T16:30:50Z

packages/aws-cdk-lib/aws-rds/test/cluster.test.ts

@@ -165,7 +187,10 @@ describe('cluster new api', () => {
      });
    });

-    test('cluster scalability option', () => {


@rix0rrr Extended our existing tests to check both the misspelled option and the correct option, for STANDARD and LIMITLESS database cluster types.

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.

github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p2 labels Jan 9, 2025

aws-cdk-automation requested a review from a team January 9, 2025 23:25

mergify bot added the contribution/core This is a PR that came from AWS. label Jan 9, 2025

iankhou changed the title ~~rds: scalability is spelled wrong~~ chore(rds): clusterScailabilityType is spelled wrong and should be clusterScalabilityType Jan 9, 2025

iankhou self-assigned this Jan 9, 2025

iankhou mentioned this pull request Jan 9, 2025

chore(rds): clusterScailabilityType is a misspelling and should be clusterScalabilityType #32820

Closed

1 task

iankhou marked this pull request as ready for review January 9, 2025 23:48

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 10, 2025

rix0rrr requested changes Jan 10, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 10, 2025

iankhou force-pushed the iankhou-rds-misspelling branch from facddcd to 0b7c820 Compare January 10, 2025 16:20

rds: scalability is spelled wrong

7328eb6

iankhou force-pushed the iankhou-rds-misspelling branch from 0b7c820 to 7328eb6 Compare January 10, 2025 16:28

iankhou commented Jan 10, 2025

View reviewed changes

iankhou changed the title ~~chore(rds): clusterScailabilityType is spelled wrong and should be clusterScalabilityType~~ fix(rds): clusterScailabilityType is spelled wrong and should be clusterScalabilityType Jan 10, 2025

aws-cdk-automation previously requested changes Jan 10, 2025

View reviewed changes

rix0rrr approved these changes Jan 10, 2025

View reviewed changes

rix0rrr added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Jan 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(rds): clusterScailabilityType is spelled wrong and should be clusterScalabilityType #32825

fix(rds): clusterScailabilityType is spelled wrong and should be clusterScalabilityType #32825

iankhou commented Jan 9, 2025

codecov bot commented Jan 9, 2025 •

edited

Loading

aws-cdk-automation commented Jan 10, 2025

rix0rrr Jan 10, 2025

iankhou Jan 10, 2025

rix0rrr Jan 10, 2025

iankhou Jan 10, 2025

iankhou Jan 10, 2025

iankhou Jan 10, 2025

aws-cdk-automation left a comment

		if (props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS) {
		if (props.clusterScalabilityType === ClusterScalabilityType.LIMITLESS \|\| props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS) {

fix(rds): clusterScailabilityType is spelled wrong and should be clusterScalabilityType #32825

Are you sure you want to change the base?

fix(rds): clusterScailabilityType is spelled wrong and should be clusterScalabilityType #32825

Conversation

iankhou commented Jan 9, 2025

Issue #32415

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

codecov bot commented Jan 9, 2025 • edited Loading

Codecov Report

aws-cdk-automation commented Jan 10, 2025

AWS CodeBuild CI Report

rix0rrr Jan 10, 2025

Choose a reason for hiding this comment

iankhou Jan 10, 2025

Choose a reason for hiding this comment

rix0rrr Jan 10, 2025

Choose a reason for hiding this comment

iankhou Jan 10, 2025

Choose a reason for hiding this comment

iankhou Jan 10, 2025

Choose a reason for hiding this comment

iankhou Jan 10, 2025

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 9, 2025 •

edited

Loading