feat(stepfunctions-tasks): add validations for EmrCreateCluster #28529

go-to-k · 2023-12-30T11:16:37Z

This PR adds validations for EmrCreateCluster.

timeoutDurationMinutes

Minimum value is 5 and maximum value is 1440.

https://docs.aws.amazon.com/emr/latest/APIReference/API_SpotProvisioningSpecification.html

bidPrice and bidPriceAsPercentageOfOnDemandPrice

Both bidPrice and bidPriceAsPercentageOfOnDemandPrice are specified, the error occurs in Step Functions console.

Specify at most one of bidPrice or bidPriceAsPercentageOfOnDemandPrice value for the Spot Instance fleet : Master request. (Service: AmazonElasticMapReduce; Status Code: 400; Error Code: ValidationException; Request ID: xxxxxx; Proxy: null)

targetOnDemandCapacity and targetSpotCapacity

At least one of TargetSpotCapacity and TargetOnDemandCapacity should be greater than 0. For a master instance fleet, only one of TargetSpotCapacity and TargetOnDemandCapacity can be specified, and its value must be 1.

https://docs.aws.amazon.com/emr/latest/APIReference/API_InstanceFleetConfig.html

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

.../integ.latest.js.snapshot/LambdaNodeJsLatestIntegDefaultTestDeployAssertD40B5C28.assets.json

aws-cdk-automation · 2023-12-30T12:41:16Z

AWS CodeBuild CI Report

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

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

kaizencc

Thanks :)

kaizencc · 2023-12-30T12:47:24Z

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/private/cluster-utils.ts

@@ -146,6 +150,9 @@ function SpotProvisioningSpecificationPropertyToJson(property?: EmrCreateCluster
  if (!property) {
    return undefined;
  }
+  if (!cdk.Token.isUnresolved(property.timeoutDurationMinutes) && (property.timeoutDurationMinutes < 5 || property.timeoutDurationMinutes > 1440)) {


Unrelated to your PR, but I can't believe we have a prop called timeoutDurationMinutes: number. It should have been typed and named as timeout: Duration. Wonder if you're willing to deprecate timeoutDurationMinutes in favor of a new prop in a separate PR

@kaizencc

Wonder if you're willing to deprecate timeoutDurationMinutes in favor of a new prop in a separate PR

OK, I will take it.

@kaizencc

Oh, this param timeoutDurationMinutes is required in the interface. ~~This means that even if we have a new parameter, we still need to specify this parameter, and making it optional would be a breaking change.~~

I was mistaken: making a required parameter optional for an interface that is a props is not a breaking change.

But making the new parameter require is a breaking change.

What should we do? Should it be added as a new optional parameter, whereas before it was required? Or do we keep going as is?

I'll submit a PR anyway.

@kaizencc

I have submitted the PR.

#28532 (comment)

mergify · 2023-12-30T12:51:02Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

…er (#28532) This PR adds a new parameter `timeout` as Duration type instead of `timeoutDurationMinutes` because the `timeoutDurationMinutes` is a number type. Originally, `timeoutDurationMinutes` was a **required** parameter, but we have made it **optional** and also made the new parameter **optional** to avoid breaking change. Instead, added a validation to ensure that the value is specified. We discussed this in the following thread: #28529 (comment) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…28529) This PR adds validations for `EmrCreateCluster`. ## timeoutDurationMinutes > Minimum value is 5 and maximum value is 1440. https://docs.aws.amazon.com/emr/latest/APIReference/API_SpotProvisioningSpecification.html ## bidPrice and bidPriceAsPercentageOfOnDemandPrice Both `bidPrice` and `bidPriceAsPercentageOfOnDemandPrice` are specified, the error occurs in Step Functions console. ``` Specify at most one of bidPrice or bidPriceAsPercentageOfOnDemandPrice value for the Spot Instance fleet : Master request. (Service: AmazonElasticMapReduce; Status Code: 400; Error Code: ValidationException; Request ID: xxxxxx; Proxy: null) ``` ## targetOnDemandCapacity and targetSpotCapacity > At least one of TargetSpotCapacity and TargetOnDemandCapacity should be greater than 0. For a master instance fleet, only one of TargetSpotCapacity and TargetOnDemandCapacity can be specified, and its value must be 1. https://docs.aws.amazon.com/emr/latest/APIReference/API_InstanceFleetConfig.html ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…er (aws#28532) This PR adds a new parameter `timeout` as Duration type instead of `timeoutDurationMinutes` because the `timeoutDurationMinutes` is a number type. Originally, `timeoutDurationMinutes` was a **required** parameter, but we have made it **optional** and also made the new parameter **optional** to avoid breaking change. Instead, added a validation to ensure that the value is specified. We discussed this in the following thread: aws#28529 (comment) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

go-to-k added 2 commits December 30, 2023 20:08

chore(stepfunctions-tasks): add validations for EmrCreateCluster

2f3d76b

tweak

5076351

aws-cdk-automation requested a review from a team December 30, 2023 11:16

github-actions bot added p2 distinguished-contributor [Pilot] contributed 50+ PRs to the CDK labels Dec 30, 2023

go-to-k added 2 commits December 30, 2023 21:13

snapshots

cce6c5b

change message

4c97b73

go-to-k commented Dec 30, 2023

View reviewed changes

.../integ.latest.js.snapshot/LambdaNodeJsLatestIntegDefaultTestDeployAssertD40B5C28.assets.json Show resolved Hide resolved

go-to-k mentioned this pull request Dec 30, 2023

test(lambda-nodejs): update snapshots #28530

Closed

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

kaizencc changed the title ~~chore(stepfunctions-tasks): add validations for EmrCreateCluster~~ feat(stepfunctions-tasks): add validations for EmrCreateCluster Dec 30, 2023

kaizencc added pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-integ-test The PR linter will not require integ test changes labels Dec 30, 2023

kaizencc approved these changes Dec 30, 2023

View reviewed changes

mergify bot merged commit e0b725c into aws:main Dec 30, 2023
26 checks passed

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

go-to-k deleted the sfn-task-emr-validation branch December 30, 2023 12:56

go-to-k mentioned this pull request Dec 30, 2023

feat(stepfunctions-tasks): add timeout parameter for EmrCreateCluster #28532

Merged

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

Monthly PR metrics report - Dec 2023 #28544

Closed

go-to-k mentioned this pull request Jan 7, 2024

feat(core): add validations for export name in CfnOutput #28575

Merged

renovate bot mentioned this pull request Mar 5, 2024

Update dependency software.amazon.awscdk:aws-cdk-lib to v2.131.0 - autoclosed cloudshiftinc/kotlin-cdk-wrapper#29

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(stepfunctions-tasks): add validations for EmrCreateCluster #28529

feat(stepfunctions-tasks): add validations for EmrCreateCluster #28529

go-to-k commented Dec 30, 2023

aws-cdk-automation commented Dec 30, 2023

kaizencc left a comment

kaizencc Dec 30, 2023

go-to-k Dec 30, 2023

go-to-k Dec 30, 2023 •

edited

go-to-k Dec 30, 2023

go-to-k Dec 30, 2023

mergify bot commented Dec 30, 2023

feat(stepfunctions-tasks): add validations for EmrCreateCluster #28529

feat(stepfunctions-tasks): add validations for EmrCreateCluster #28529

Conversation

go-to-k commented Dec 30, 2023

timeoutDurationMinutes

bidPrice and bidPriceAsPercentageOfOnDemandPrice

targetOnDemandCapacity and targetSpotCapacity

aws-cdk-automation commented Dec 30, 2023

AWS CodeBuild CI Report

kaizencc left a comment

Choose a reason for hiding this comment

kaizencc Dec 30, 2023

Choose a reason for hiding this comment

go-to-k Dec 30, 2023

Choose a reason for hiding this comment

go-to-k Dec 30, 2023 • edited

Choose a reason for hiding this comment

go-to-k Dec 30, 2023

Choose a reason for hiding this comment

go-to-k Dec 30, 2023

Choose a reason for hiding this comment

mergify bot commented Dec 30, 2023

go-to-k Dec 30, 2023 •

edited