From ed0bb6ec37e1193fcbfd73be9ef7aca69c5a546f Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sat, 30 Dec 2023 22:57:12 +0900 Subject: [PATCH 1/6] feat(stepfunctions-tasks): add parameter as Duration type --- ...create-cluster-with-spot-instance-fleet.ts | 4 +- .../aws-stepfunctions-tasks/README.md | 2 +- .../lib/emr/emr-create-cluster.ts | 15 ++- .../lib/emr/private/cluster-utils.ts | 18 +++- .../test/emr/emr-create-cluster.test.ts | 95 +++++++++++++++++-- 5 files changed, 120 insertions(+), 14 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-with-spot-instance-fleet.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-with-spot-instance-fleet.ts index ea24bac23c2d9..ecbdf872a134f 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-with-spot-instance-fleet.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-with-spot-instance-fleet.ts @@ -1,5 +1,5 @@ import * as sfn from 'aws-cdk-lib/aws-stepfunctions'; -import { App, Stack } from 'aws-cdk-lib'; +import { App, Duration, Stack } from 'aws-cdk-lib'; import { IntegTest } from '@aws-cdk/integ-tests-alpha'; import { EmrCreateCluster } from 'aws-cdk-lib/aws-stepfunctions-tasks'; @@ -20,7 +20,7 @@ const step = new EmrCreateCluster(stack, 'EmrCreateCluster', { spotSpecification: { allocationStrategy: EmrCreateCluster.SpotAllocationStrategy.CAPACITY_OPTIMIZED, timeoutAction: EmrCreateCluster.SpotTimeoutAction.TERMINATE_CLUSTER, - timeoutDurationMinutes: 60, + timeout: Duration.minutes(60), }, }, name: 'Master', diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md b/packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md index f142c4890be2b..c19d860f46892 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md @@ -634,7 +634,7 @@ new tasks.EmrCreateCluster(this, 'SpotSpecification', { spotSpecification: { allocationStrategy: tasks.EmrCreateCluster.SpotAllocationStrategy.CAPACITY_OPTIMIZED, timeoutAction: tasks.EmrCreateCluster.SpotTimeoutAction.TERMINATE_CLUSTER, - timeoutDurationMinutes: 60, + timeout: Duration.minutes(5), }, }, }], diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts index 46d6f79a06704..66272a8af966d 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts @@ -732,9 +732,20 @@ export namespace EmrCreateCluster { /** * The spot provisioning timeout period in minutes. * - * The value must be between 5 and 1440. + * The value must be between 5 and 1440 minutes. + * + * @deprecated - Use `timeout`. + */ + readonly timeoutDurationMinutes?: number; + + /** + * The spot provisioning timeout period in minutes. + * + * The value must be between 5 and 1440 minutes. + * + * You must specify one of `timeout` and `timeoutDurationMinutes`. */ - readonly timeoutDurationMinutes: number; + readonly timeout?: cdk.Duration; } /** diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts index 4906055e0389b..69289541031e0 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts @@ -150,14 +150,26 @@ function SpotProvisioningSpecificationPropertyToJson(property?: EmrCreateCluster if (!property) { return undefined; } - if (!cdk.Token.isUnresolved(property.timeoutDurationMinutes) && (property.timeoutDurationMinutes < 5 || property.timeoutDurationMinutes > 1440)) { - throw new Error(`timeoutDurationMinutes must be between 5 and 1440, got ${property.timeoutDurationMinutes}`); + + if (!property.timeout && !property.timeoutDurationMinutes) { + throw new Error('timeout must be specified'); + } + if (property.timeout && (property.timeout.toMinutes() < 5 || property.timeout.toMinutes() > 1440)) { + throw new Error(`timeout must be between 5 and 1440 minutes, got ${property.timeout.toMinutes()}`); } + if ( + property.timeoutDurationMinutes + && !cdk.Token.isUnresolved(property.timeoutDurationMinutes) + && (property.timeoutDurationMinutes < 5 || property.timeoutDurationMinutes > 1440) + ) { + throw new Error(`timeoutDurationMinutes must be between 5 and 1440 minutes, got ${property.timeoutDurationMinutes}`); + } + return { AllocationStrategy: cdk.stringToCloudFormation(property.allocationStrategy), BlockDurationMinutes: cdk.numberToCloudFormation(property.blockDurationMinutes), TimeoutAction: cdk.stringToCloudFormation(property.timeoutAction?.valueOf()), - TimeoutDurationMinutes: cdk.numberToCloudFormation(property.timeoutDurationMinutes), + TimeoutDurationMinutes: cdk.numberToCloudFormation(property.timeout?.toMinutes() || property.timeoutDurationMinutes), }; } diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts index 69f9044056ca0..606ff5431681f 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts @@ -921,7 +921,7 @@ test.each([ allocationStrategy: strategy, blockDurationMinutes: 1, timeoutAction: EmrCreateCluster.SpotTimeoutAction.TERMINATE_CLUSTER, - timeoutDurationMinutes: 5, + timeout: cdk.Duration.minutes(5), }, }, name: 'Main', @@ -1033,7 +1033,7 @@ test('Create Cluster with InstanceFleet for Spot instances', () => { spotSpecification: { blockDurationMinutes: 1, timeoutAction: EmrCreateCluster.SpotTimeoutAction.TERMINATE_CLUSTER, - timeoutDurationMinutes: 5, + timeout: cdk.Duration.minutes(5), }, }, name: 'Main', @@ -1219,7 +1219,63 @@ test('Create Cluster with InstanceFleet for On-Demand instances', () => { }); }); -test('Throws if timeoutDurationMinutes for Spot instances is less than 5', () => { +test('Throws if timeout for Spot instances is less than 5 minutes', () => { + // GIVEN + const task = new EmrCreateCluster(stack, 'Task', { + instances: { + instanceFleets: [{ + instanceFleetType: EmrCreateCluster.InstanceRoleType.MASTER, + launchSpecifications: { + spotSpecification: { + timeoutAction: EmrCreateCluster.SpotTimeoutAction.TERMINATE_CLUSTER, + timeout: cdk.Duration.minutes(4), + }, + }, + name: 'Main', + targetSpotCapacity: 1, + }], + }, + clusterRole, + name: 'Cluster', + serviceRole, + integrationPattern: sfn.IntegrationPattern.REQUEST_RESPONSE, + }); + + // THEN + expect(() => { + task.toStateJson(); + }).toThrow(/timeout must be between 5 and 1440 minutes, got 4/); +}); + +test('Throws if timeout for Spot instances is greater than 1440 minutes', () => { + // GIVEN + const task = new EmrCreateCluster(stack, 'Task', { + instances: { + instanceFleets: [{ + instanceFleetType: EmrCreateCluster.InstanceRoleType.MASTER, + launchSpecifications: { + spotSpecification: { + timeoutAction: EmrCreateCluster.SpotTimeoutAction.TERMINATE_CLUSTER, + timeout: cdk.Duration.minutes(1441), + }, + }, + name: 'Main', + targetSpotCapacity: 1, + }], + }, + clusterRole, + name: 'Cluster', + serviceRole, + integrationPattern: sfn.IntegrationPattern.REQUEST_RESPONSE, + }); + + // THEN + expect(() => { + task.toStateJson(); + }).toThrow(/timeout must be between 5 and 1440 minutes, got 1441/); +}); + +test('Throws if timeoutDurationMinutes for Spot instances is less than 5 minutes', () => { // GIVEN const task = new EmrCreateCluster(stack, 'Task', { instances: { @@ -1244,10 +1300,10 @@ test('Throws if timeoutDurationMinutes for Spot instances is less than 5', () => // THEN expect(() => { task.toStateJson(); - }).toThrow(/timeoutDurationMinutes must be between 5 and 1440, got 4/); + }).toThrow(/timeoutDurationMinutes must be between 5 and 1440 minutes, got 4/); }); -test('Throws if timeoutDurationMinutes for Spot instances is greater than 1440', () => { +test('Throws if timeoutDurationMinutes for Spot instances is greater than 1440 minutes', () => { // GIVEN const task = new EmrCreateCluster(stack, 'Task', { instances: { @@ -1272,7 +1328,34 @@ test('Throws if timeoutDurationMinutes for Spot instances is greater than 1440', // THEN expect(() => { task.toStateJson(); - }).toThrow(/timeoutDurationMinutes must be between 5 and 1440, got 1441/); + }).toThrow(/timeoutDurationMinutes must be between 5 and 1440 minutes, got 1441/); +}); + +test('Throws if both timeout and timeoutDurationMinutes are not specified', () => { + // GIVEN + const task = new EmrCreateCluster(stack, 'Task', { + instances: { + instanceFleets: [{ + instanceFleetType: EmrCreateCluster.InstanceRoleType.MASTER, + launchSpecifications: { + spotSpecification: { + timeoutAction: EmrCreateCluster.SpotTimeoutAction.TERMINATE_CLUSTER, + }, + }, + name: 'Main', + targetSpotCapacity: 1, + }], + }, + clusterRole, + name: 'Cluster', + serviceRole, + integrationPattern: sfn.IntegrationPattern.REQUEST_RESPONSE, + }); + + // THEN + expect(() => { + task.toStateJson(); + }).toThrow(/timeout must be specified/); }); test('Throws if both bidPrice and bidPriceAsPercentageOfOnDemandPrice are specified', () => { From e06ab3960c620cfa6f9e116764dadb8323d78e94 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sat, 30 Dec 2023 23:14:45 +0900 Subject: [PATCH 2/6] change docs --- .../aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts index 66272a8af966d..af9a404f6815c 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts @@ -734,6 +734,8 @@ export namespace EmrCreateCluster { * * The value must be between 5 and 1440 minutes. * + * @default - No timeoutDurationMinutes + * * @deprecated - Use `timeout`. */ readonly timeoutDurationMinutes?: number; @@ -744,6 +746,8 @@ export namespace EmrCreateCluster { * The value must be between 5 and 1440 minutes. * * You must specify one of `timeout` and `timeoutDurationMinutes`. + * + * @default - No timeout */ readonly timeout?: cdk.Duration; } From f3840cc8f9f9e5dfb63a65d5944b7e6bfae14534 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sat, 30 Dec 2023 23:51:58 +0900 Subject: [PATCH 3/6] change branch condition --- .../aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts index 69289541031e0..a63c9becf7ed2 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts @@ -158,7 +158,8 @@ function SpotProvisioningSpecificationPropertyToJson(property?: EmrCreateCluster throw new Error(`timeout must be between 5 and 1440 minutes, got ${property.timeout.toMinutes()}`); } if ( - property.timeoutDurationMinutes + !property.timeout // ignore validation because `timeoutDurationMinutes` is not used if a `timeout` is specified + && property.timeoutDurationMinutes && !cdk.Token.isUnresolved(property.timeoutDurationMinutes) && (property.timeoutDurationMinutes < 5 || property.timeoutDurationMinutes > 1440) ) { From 45b4a1bdc02366cdc2d3c2cd9c320e4f84c3fd16 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 31 Dec 2023 00:23:08 +0900 Subject: [PATCH 4/6] change unit tests --- .../test/emr/emr-create-cluster.test.ts | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts index 606ff5431681f..b4a8503f9a7b3 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts @@ -1331,7 +1331,7 @@ test('Throws if timeoutDurationMinutes for Spot instances is greater than 1440 m }).toThrow(/timeoutDurationMinutes must be between 5 and 1440 minutes, got 1441/); }); -test('Throws if both timeout and timeoutDurationMinutes are not specified', () => { +test('Throws if neither timeout nor timeoutDurationMinutes is specified', () => { // GIVEN const task = new EmrCreateCluster(stack, 'Task', { instances: { @@ -1358,6 +1358,72 @@ test('Throws if both timeout and timeoutDurationMinutes are not specified', () = }).toThrow(/timeout must be specified/); }); +test('timeout takes precedence if both timeout and timeoutDurationMinutes are specified', () => { + // WHEN + const task = new EmrCreateCluster(stack, 'Task', { + instances: { + instanceFleets: [{ + instanceFleetType: EmrCreateCluster.InstanceRoleType.MASTER, + launchSpecifications: { + spotSpecification: { + timeoutAction: EmrCreateCluster.SpotTimeoutAction.TERMINATE_CLUSTER, + timeout: cdk.Duration.minutes(5), + timeoutDurationMinutes: 10, + }, + }, + name: 'Main', + targetSpotCapacity: 1, + }], + }, + clusterRole, + name: 'Cluster', + serviceRole, + integrationPattern: sfn.IntegrationPattern.REQUEST_RESPONSE, + }); + + // THEN + expect(stack.resolve(task.toStateJson())).toEqual({ + Type: 'Task', + Resource: { + 'Fn::Join': [ + '', + [ + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':states:::elasticmapreduce:createCluster', + ], + ], + }, + End: true, + Parameters: { + Name: 'Cluster', + Instances: { + KeepJobFlowAliveWhenNoSteps: true, + InstanceFleets: [{ + InstanceFleetType: 'MASTER', + LaunchSpecifications: { + SpotSpecification: { + TimeoutAction: 'TERMINATE_CLUSTER', + TimeoutDurationMinutes: 5, + }, + }, + Name: 'Main', + TargetSpotCapacity: 1, + }], + }, + VisibleToAllUsers: true, + JobFlowRole: { + Ref: 'ClusterRoleD9CA7471', + }, + ServiceRole: { + Ref: 'ServiceRole4288B192', + }, + }, + }); +}); + test('Throws if both bidPrice and bidPriceAsPercentageOfOnDemandPrice are specified', () => { // GIVEN const task = new EmrCreateCluster(stack, 'Task', { From ad29524f92bccc06444ac594fcf2e0de46743af3 Mon Sep 17 00:00:00 2001 From: go-to-k <24818752+go-to-k@users.noreply.github.com> Date: Sun, 31 Dec 2023 12:59:52 +0900 Subject: [PATCH 5/6] change by review tweak --- .../lib/emr/emr-create-cluster.ts | 6 ++- .../lib/emr/private/cluster-utils.ts | 17 ++----- .../test/emr/emr-create-cluster.test.ts | 51 +++---------------- 3 files changed, 16 insertions(+), 58 deletions(-) diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts index af9a404f6815c..0f1feb0a096fb 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts @@ -734,7 +734,9 @@ export namespace EmrCreateCluster { * * The value must be between 5 and 1440 minutes. * - * @default - No timeoutDurationMinutes + * You must specify one of `timeout` and `timeoutDurationMinutes`. + * + * @default - The value in `timeout` is used * * @deprecated - Use `timeout`. */ @@ -747,7 +749,7 @@ export namespace EmrCreateCluster { * * You must specify one of `timeout` and `timeoutDurationMinutes`. * - * @default - No timeout + * @default - The value in `timeoutDurationMinutes` is used */ readonly timeout?: cdk.Duration; } diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts index a63c9becf7ed2..ad33648e21ebe 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts @@ -151,19 +151,12 @@ function SpotProvisioningSpecificationPropertyToJson(property?: EmrCreateCluster return undefined; } - if (!property.timeout && !property.timeoutDurationMinutes) { - throw new Error('timeout must be specified'); + if ((property.timeout && property.timeoutDurationMinutes) || (!property.timeout && !property.timeoutDurationMinutes)) { + throw new Error('one of timeout and timeoutDurationMinutes must be specified'); } - if (property.timeout && (property.timeout.toMinutes() < 5 || property.timeout.toMinutes() > 1440)) { - throw new Error(`timeout must be between 5 and 1440 minutes, got ${property.timeout.toMinutes()}`); - } - if ( - !property.timeout // ignore validation because `timeoutDurationMinutes` is not used if a `timeout` is specified - && property.timeoutDurationMinutes - && !cdk.Token.isUnresolved(property.timeoutDurationMinutes) - && (property.timeoutDurationMinutes < 5 || property.timeoutDurationMinutes > 1440) - ) { - throw new Error(`timeoutDurationMinutes must be between 5 and 1440 minutes, got ${property.timeoutDurationMinutes}`); + const timeout = property.timeout?.toMinutes() ?? property.timeoutDurationMinutes; + if (timeout !== undefined && !cdk.Token.isUnresolved(timeout) && (timeout < 5 || timeout > 1440)) { + throw new Error(`timeout must be between 5 and 1440 minutes, got ${timeout}`); } return { diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts index b4a8503f9a7b3..94a65e8c767b9 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts @@ -1300,7 +1300,7 @@ test('Throws if timeoutDurationMinutes for Spot instances is less than 5 minutes // THEN expect(() => { task.toStateJson(); - }).toThrow(/timeoutDurationMinutes must be between 5 and 1440 minutes, got 4/); + }).toThrow(/timeout must be between 5 and 1440 minutes, got 4/); }); test('Throws if timeoutDurationMinutes for Spot instances is greater than 1440 minutes', () => { @@ -1328,7 +1328,7 @@ test('Throws if timeoutDurationMinutes for Spot instances is greater than 1440 m // THEN expect(() => { task.toStateJson(); - }).toThrow(/timeoutDurationMinutes must be between 5 and 1440 minutes, got 1441/); + }).toThrow(/timeout must be between 5 and 1440 minutes, got 1441/); }); test('Throws if neither timeout nor timeoutDurationMinutes is specified', () => { @@ -1355,10 +1355,10 @@ test('Throws if neither timeout nor timeoutDurationMinutes is specified', () => // THEN expect(() => { task.toStateJson(); - }).toThrow(/timeout must be specified/); + }).toThrow(/one of timeout and timeoutDurationMinutes must be specified/); }); -test('timeout takes precedence if both timeout and timeoutDurationMinutes are specified', () => { +test('Throws if both timeout and timeoutDurationMinutes are specified', () => { // WHEN const task = new EmrCreateCluster(stack, 'Task', { instances: { @@ -1382,46 +1382,9 @@ test('timeout takes precedence if both timeout and timeoutDurationMinutes are sp }); // THEN - expect(stack.resolve(task.toStateJson())).toEqual({ - Type: 'Task', - Resource: { - 'Fn::Join': [ - '', - [ - 'arn:', - { - Ref: 'AWS::Partition', - }, - ':states:::elasticmapreduce:createCluster', - ], - ], - }, - End: true, - Parameters: { - Name: 'Cluster', - Instances: { - KeepJobFlowAliveWhenNoSteps: true, - InstanceFleets: [{ - InstanceFleetType: 'MASTER', - LaunchSpecifications: { - SpotSpecification: { - TimeoutAction: 'TERMINATE_CLUSTER', - TimeoutDurationMinutes: 5, - }, - }, - Name: 'Main', - TargetSpotCapacity: 1, - }], - }, - VisibleToAllUsers: true, - JobFlowRole: { - Ref: 'ClusterRoleD9CA7471', - }, - ServiceRole: { - Ref: 'ServiceRole4288B192', - }, - }, - }); + expect(() => { + task.toStateJson(); + }).toThrow(/one of timeout and timeoutDurationMinutes must be specified/); }); test('Throws if both bidPrice and bidPriceAsPercentageOfOnDemandPrice are specified', () => { From b42f6c80c5386151b9b4cb12ec15b05d6cdc5bf4 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com> Date: Tue, 2 Jan 2024 12:15:24 +0100 Subject: [PATCH 6/6] Apply suggestions from code review --- .../lib/emr/private/cluster-utils.ts | 2 +- .../test/emr/emr-create-cluster.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts index ad33648e21ebe..21630dd0efe4e 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts @@ -156,7 +156,7 @@ function SpotProvisioningSpecificationPropertyToJson(property?: EmrCreateCluster } const timeout = property.timeout?.toMinutes() ?? property.timeoutDurationMinutes; if (timeout !== undefined && !cdk.Token.isUnresolved(timeout) && (timeout < 5 || timeout > 1440)) { - throw new Error(`timeout must be between 5 and 1440 minutes, got ${timeout}`); + throw new Error(`timeout must be between 5 and 1440 minutes, got ${timeout} minutes.`); } return { diff --git a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts index 94a65e8c767b9..5c3b69a412087 100644 --- a/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts +++ b/packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts @@ -1272,7 +1272,7 @@ test('Throws if timeout for Spot instances is greater than 1440 minutes', () => // THEN expect(() => { task.toStateJson(); - }).toThrow(/timeout must be between 5 and 1440 minutes, got 1441/); + }).toThrow(/timeout must be between 5 and 1440 minutes, got 1441 minutes./); }); test('Throws if timeoutDurationMinutes for Spot instances is less than 5 minutes', () => { @@ -1300,7 +1300,7 @@ test('Throws if timeoutDurationMinutes for Spot instances is less than 5 minutes // THEN expect(() => { task.toStateJson(); - }).toThrow(/timeout must be between 5 and 1440 minutes, got 4/); + }).toThrow(/timeout must be between 5 and 1440 minutes, got 4 minutes./); }); test('Throws if timeoutDurationMinutes for Spot instances is greater than 1440 minutes', () => { @@ -1328,7 +1328,7 @@ test('Throws if timeoutDurationMinutes for Spot instances is greater than 1440 m // THEN expect(() => { task.toStateJson(); - }).toThrow(/timeout must be between 5 and 1440 minutes, got 1441/); + }).toThrow(/timeout must be between 5 and 1440 minutes, got 1441 minutes./); }); test('Throws if neither timeout nor timeoutDurationMinutes is specified', () => {