Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Merged
merged 2 commits into from
Jan 10, 2025
+69 −8
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7328eb6
rds: scalability is spelled wrong
iankhou Jan 9, 2025
01f6f5c
Merge branch 'main' into iankhou-rds-misspelling
mergify[bot] Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-rds/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1436,7 +1436,7 @@ new rds.DatabaseCluster(this, 'LimitlessDatabaseCluster', {
version: rds.AuroraPostgresEngineVersion.VER_16_4_LIMITLESS,
}),
vpc,
clusterScailabilityType: rds.ClusterScailabilityType.LIMITLESS,
clusterScalabilityType: rds.ClusterScalabilityType.LIMITLESS,
// Requires enabling Performance Insights
enablePerformanceInsights: true,
performanceInsightRetention: rds.PerformanceInsightRetention.MONTHS_1,
Expand Down
39 changes: 36 additions & 3 deletions packages/aws-cdk-lib/aws-rds/lib/cluster.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,17 @@ interface DatabaseClusterBaseProps {
*
* Set LIMITLESS if you want to use a limitless database; otherwise, set it to STANDARD.
*
* @default ClusterScalabilityType.STANDARD
*/
readonly clusterScalabilityType?: ClusterScalabilityType;

/**
* [Misspelled] Specifies the scalability mode of the Aurora DB cluster.
*
* Set LIMITLESS if you want to use a limitless database; otherwise, set it to STANDARD.
*
* @default ClusterScailabilityType.STANDARD
* @deprecated Use clusterScalabilityType instead. This will be removed in the next major version.
*/
readonly clusterScailabilityType?: ClusterScailabilityType;
}
Expand Down Expand Up @@ -492,6 +502,25 @@ export enum InstanceUpdateBehaviour {
/**
* The scalability mode of the Aurora DB cluster.
*/
export enum ClusterScalabilityType {
/**
* The cluster uses normal DB instance creation.
*/
STANDARD = 'standard',

/**
* The cluster operates as an Aurora Limitless Database,
* allowing you to create a DB shard group for horizontal scaling (sharding) capabilities.
*
* @see https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/limitless.html
*/
LIMITLESS = 'limitless',
}

/**
* The scalability mode of the Aurora DB cluster.
* @deprecated Use ClusterScalabilityType instead. This will be removed in the next major version.
*/
export enum ClusterScailabilityType {
/**
* The cluster uses normal DB instance creation.
Expand Down Expand Up @@ -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.');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rix0rrr here I disallow both options to be set.

}

if ((props.vpc && props.instanceProps?.vpc) || (!props.vpc && !props.instanceProps?.vpc)) {
throw new Error('Provide either vpc or instanceProps.vpc, but not both');
}
Expand Down Expand Up @@ -802,7 +835,7 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
throw new Error('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set');
}

if (props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS) {
if (props.clusterScalabilityType === ClusterScalabilityType.LIMITLESS || props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS) {
Comment on lines -805 to +838
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will make this change.

if (!props.enablePerformanceInsights) {
throw new Error('Performance Insights must be enabled for Aurora Limitless Database.');
}
Expand Down Expand Up @@ -877,7 +910,7 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
}),
storageType: props.storageType?.toString(),
enableLocalWriteForwarding: props.enableLocalWriteForwarding,
clusterScalabilityType: props.clusterScailabilityType,
clusterScalabilityType: props.clusterScalabilityType ?? props.clusterScailabilityType,
// Admin
backtrackWindow: props.backtrackWindow?.toSeconds(),
backupRetentionPeriod: props.backup?.retention?.toDays(),
Expand Down Expand Up @@ -1287,7 +1320,7 @@ export class DatabaseCluster extends DatabaseClusterNew {
setLogRetention(this, props);

// create the instances for only standard aurora clusters
if (props.clusterScailabilityType !== ClusterScailabilityType.LIMITLESS) {
if (props.clusterScalabilityType !== ClusterScalabilityType.LIMITLESS && props.clusterScailabilityType !== ClusterScailabilityType.LIMITLESS) {
if ((props.writer || props.readers) && (props.instances || props.instanceProps)) {
throw new Error('Cannot provide writer or readers if instances or instanceProps are provided');
}
Expand Down
36 changes: 32 additions & 4 deletions packages/aws-cdk-lib/aws-rds/test/cluster.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,34 @@ import {
DatabaseClusterEngine, DatabaseClusterFromSnapshot, ParameterGroup, PerformanceInsightRetention, SubnetGroup, DatabaseSecret,
DatabaseInstanceEngine, SqlServerEngineVersion, SnapshotCredentials, InstanceUpdateBehaviour, NetworkType, ClusterInstance, CaCertificate,
IClusterEngine,
ClusterScalabilityType,
ClusterScailabilityType,
DBClusterStorageType,
} from '../lib';

describe('cluster new api', () => {
describe('errors are thrown', () => {
test('when both clusterScalabilityType and clusterScailabilityType (deprecated) props are provided', () => {
// GIVEN
const stack = testStack();
const vpc = new ec2.Vpc(stack, 'VPC');

expect(() => {
// WHEN
new DatabaseCluster(stack, 'Database', {
engine: DatabaseClusterEngine.AURORA_MYSQL,
instanceProps: {
instanceType: ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE2, ec2.InstanceSize.SMALL),
vpc,
},
clusterScalabilityType: ClusterScalabilityType.STANDARD,
clusterScailabilityType: ClusterScailabilityType.STANDARD,
iamAuthentication: true,
});
// THEN
}).toThrow('You cannot specify both clusterScalabilityType and clusterScailabilityType (deprecated). Use clusterScalabilityType.');
});

test('when old and new props are provided', () => {
// GIVEN
const stack = testStack();
Expand Down Expand Up @@ -165,7 +187,10 @@ describe('cluster new api', () => {
});
});

test('cluster scalability option', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

test.each([
['clusterScalabilityType', 'clusterScalabilityType', ClusterScalabilityType.STANDARD],
['clusterScailabilityType (deprecated)', 'clusterScailabilityType', ClusterScailabilityType.STANDARD],
])('cluster scalability option with %s', (_, propName, propValue) => {
// GIVEN
const stack = testStack();
const vpc = new ec2.Vpc(stack, 'VPC');
Expand All @@ -174,8 +199,8 @@ describe('cluster new api', () => {
new DatabaseCluster(stack, 'Cluster', {
engine: DatabaseClusterEngine.AURORA_MYSQL,
vpc,
clusterScailabilityType: ClusterScailabilityType.STANDARD,
writer: ClusterInstance.serverlessV2('writer'),
[propName]: propValue,
});

// THEN
Expand All @@ -186,7 +211,10 @@ describe('cluster new api', () => {
});

describe('limitless database', () => {
test('with default options', () => {
test.each([
['clusterScalabilityType', 'clusterScalabilityType', ClusterScalabilityType.LIMITLESS],
['clusterScailabilityType (deprecated)', 'clusterScailabilityType', ClusterScailabilityType.LIMITLESS],
])('with default options using %s', (_, propName, propValue) => {
// GIVEN
const stack = testStack();
const vpc = new ec2.Vpc(stack, 'VPC');
Expand All @@ -197,7 +225,7 @@ describe('cluster new api', () => {
version: AuroraPostgresEngineVersion.VER_16_4_LIMITLESS,
}),
vpc,
clusterScailabilityType: ClusterScailabilityType.LIMITLESS,
[propName]: propValue,
enablePerformanceInsights: true,
performanceInsightRetention: PerformanceInsightRetention.MONTHS_1,
monitoringInterval: cdk.Duration.minutes(1),
Expand Down
Loading