-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(stepfunctions-tasks): add step concurrency level to EmrCreateCluster #15242
feat(stepfunctions-tasks): add step concurrency level to EmrCreateCluster #15242
Conversation
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MousaZeidBaker thanks for opening a contribution.
I think we also need to add some tests to go with this added feature.
packages/@aws-cdk/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts
Outdated
Show resolved
Hide resolved
Thanks for your comments @shivlaks. They should now all be resolved. |
packages/@aws-cdk/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts
Outdated
Show resolved
Hide resolved
…luster.ts Co-authored-by: Ben Chaimberg <youppi3@gmail.com>
…luster.ts Co-authored-by: Ben Chaimberg <youppi3@gmail.com>
…luster.ts Co-authored-by: Ben Chaimberg <youppi3@gmail.com>
Pull request has been modified.
…luster.ts Co-authored-by: Ben Chaimberg <youppi3@gmail.com>
Thanks for the suggestions @BenChaimberg, they should now all be committed. Do you know why the AWS CodeBuild is failing? |
Click "Build Logs" on the aws-cdk-automation comment. You have a compilation error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a small blurb to the README
packages/@aws-cdk/aws-stepfunctions-tasks/test/emr/emr-create-cluster.test.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-stepfunctions-tasks/lib/emr/emr-create-cluster.ts
Outdated
Show resolved
Hide resolved
For those who have been waiting for this feature by accident, they could consider implementing the following class in advance: import * as sfn from '@aws-cdk/aws-stepfunctions';
import * as tasks from '@aws-cdk/aws-stepfunctions-tasks';
interface ExtendedEmrCreateClusterProps extends tasks.EmrCreateClusterProps {
/**
* Specifies the step concurrency level to allow multiple steps to run in parallel
*
* Requires EMR release label 5.28.0 or above.
* Must be in range [1, 256].
*
* @default 1 - no step concurrency allowed
*/
readonly stepConcurrencyLevel?: number;
}
/**
* A Step Functions Task to create an EMR Cluster.
*
* The ClusterConfiguration is defined as Parameters in the state machine definition.
*
* OUTPUT: the ClusterId.
*
* @see https://github.com/aws/aws-cdk/issues/14408
* @see https://github.com/aws/aws-cdk/issues/15223
* @see https://github.com/aws/aws-cdk/pull/15242/
*/
class ExtendedEmrCreateCluster extends tasks.EmrCreateCluster {
protected readonly stepConcurrencyLevel: number;
constructor(scope: cdk.Construct, id: string, props: ExtendedEmrCreateClusterProps) {
super(scope, id, props);
this.stepConcurrencyLevel = props.stepConcurrencyLevel ?? 1;
}
protected _renderTask(): any {
const originalObject = super._renderTask();
const extensionObject = {};
Object.assign(extensionObject, originalObject, { Parameters: { StepConcurrencyLevel: cdk.numberToCloudFormation(this.stepConcurrencyLevel), ...originalObject.Parameters } })
return extensionObject
}
} |
924c117
to
ebfd5f2
Compare
Cleaning up a stale PR inherited from @BenChaimberg. I can't request your review Ben, but I'm happy to take comments if you have them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small comment, rest looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Nice work, @kaizen3031593 and @MousaZeidBaker!
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
* '15588' of https://github.com/xykkong/aws-cdk: (47 commits) chore: rollback `GenericSSMParameterImage` deprecation (backport aws#16798) (aws#16800) chore(deps): bump actions/setup-node from 2.4.0 to 2.4.1 (aws#16778) Update CHANGELOG.md chore(release): 1.126.0 feat(assertions): matcher support for `templateMatches()` API (aws#16789) feat(stepfunctions-tasks): add step concurrency level to EmrCreateCluster (aws#15242) docs(s3): correct heading levels Object Ownership / Bucket deletion (aws#16790) chore(individual-pkg-gen): fix bug in setting alpha package visibility (aws#16787) fix(s3): setting `autoDeleteObjects` to `false` empties the bucket (aws#16756) fix(iam): `User.fromUserArn` does not work for ARNs that include a path (aws#16269) fix(cli): progress bar overshoots count by 1 for stack updates (aws#16168) fix(config): add SourceAccount condition to Lambda permission (aws#16617) docs(events): add a note about not using `EventPattern` with `CfnRule` (aws#16715) docs(core): fix reference to nonexistant enum value (aws#16716) chore(s3-deployments): update python version on BucketDeployment handler (aws#16771) chore: set response-requested length to 2 and closing-soon to 5 (aws#16763) fix(revert): "fix: CDK does not honor NO_PROXY settings (aws#16751)" (aws#16761) docs(GitHub issue templates): Upgrade to GitHub Issues v2 (aws#16592) chore: reset jsii-rosetta worker count to default (aws#16755) fix: CDK does not honor NO_PROXY settings (aws#16751) ...
…ster (#15242) Added support for step concurrency when creating EMR clusters through Step Functions. This feature allows users to run multiple steps in parallel on a cluster created through SFN. closes #15223. As a byproduct, adds validation for `releaseLabel` to ensure that it follows the correct format laid out [here](https://docs.aws.amazon.com/emr/latest/ReleaseGuide/emr-release-components.html). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ster (aws#15242) Added support for step concurrency when creating EMR clusters through Step Functions. This feature allows users to run multiple steps in parallel on a cluster created through SFN. closes aws#15223. As a byproduct, adds validation for `releaseLabel` to ensure that it follows the correct format laid out [here](https://docs.aws.amazon.com/emr/latest/ReleaseGuide/emr-release-components.html). ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Added support for step concurrency when creating EMR clusters through Step Functions. This feature allows users to run multiple steps in parallel on a cluster created through SFN.
closes #15223.
As a byproduct, adds validation for
releaseLabel
to ensure that it follows the correct format laid out here.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license