feat(stepfunctions): support Map ItemProcessor by lpizzinidev · Pull Request #27913 · aws/aws-cdk

lpizzinidev · 2023-11-09T16:51:51Z

Adds support for Map's ItemProcessor required field and deprecates Iterator.

Closes #27878.

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

packages/aws-cdk-lib/aws-stepfunctions/lib/states/state.ts

kaizencc

Some preliminary thoughts @lpizzinidev, but thanks for getting this started!

kaizencc · 2023-12-13T21:36:07Z

packages/aws-cdk-lib/aws-stepfunctions/README.md

+map.itemProcessor(definition);
 ```

+To define a distributed `Map` state use the configuration parameter.


unclear to me what the "configuration parameter" is

kaizencc · 2023-12-13T21:37:40Z

packages/aws-cdk-lib/aws-stepfunctions/lib/states/map.ts


-    if (this.iteration === undefined) {
-      errors.push('Map state must have a non-empty iterator');
+    if (this.iteration === undefined && this.processor === undefined) {


I mean, this is fine, but why not if (!this.iteration && !this.processor)

kaizencc · 2023-12-13T21:38:07Z

packages/aws-cdk-lib/aws-stepfunctions/lib/states/map.ts

+   *
+   * @deprecated - use `itemProcessor` instead.
   */
  public iterator(iterator: IChainable): Map {


Docstring should mention mutex with itemProcessor

kaizencc · 2023-12-13T21:38:16Z

packages/aws-cdk-lib/aws-stepfunctions/lib/states/map.ts

+  /**
+   * Define item processor in Map
+   */
+  public itemProcessor(processor: IChainable, config: ProcessorConfig = {}): Map {


Docstring should mention mutex with iterator

kaizencc · 2023-12-13T21:39:54Z

packages/aws-cdk-lib/aws-stepfunctions/lib/states/state.ts

+    if (!this.iteration) return undefined;
    return {
-      Iterator: this.iteration.toGraphJson(),
+      Iterator: this.iteration?.toGraphJson(),


don't need this ? right? typescript should know whats up based on line 430

kaizencc · 2023-12-13T21:40:27Z

packages/aws-cdk-lib/aws-stepfunctions/lib/states/state.ts

+    return {
+      ItemProcessor: {
+        ...this.renderProcessorConfig(),
+        ...this.processor?.toGraphJson(),


Shouldn't TS know this.processor exists without the ??

kaizencc · 2023-12-13T21:40:59Z

packages/aws-cdk-lib/aws-stepfunctions/lib/states/state.ts

+  /**
+   * Render ProcessorConfig in ASL JSON format
+   */
+  private renderProcessorConfig(): any {


Suggested change

private renderProcessorConfig(): any {

private renderProcessorConfig() {

any doesn't help much, i'd rather TS guses at the return type

Makes sense, I specified the return type as this is the approach used for the other rendering functions.

Fair :). If there's a reason for it I can't think of one, but it's whatever either way.

Pull request has been modified.

mergify · 2023-12-14T23:38:58Z

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).

mergify · 2023-12-14T23:42:52Z

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).

aws-cdk-automation · 2023-12-15T00:24:13Z

AWS CodeBuild CI Report

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

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

mergify · 2023-12-15T00:24:39Z

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).

Adds support for Map's [ItemSelector](https://docs.aws.amazon.com/step-functions/latest/dg/input-output-itemselector.html) field and deprecates [parameters](https://docs.aws.amazon.com/step-functions/latest/dg/concepts-asl-use-map-state-inline.html#map-state-inline-deprecated-fields). With the release of Distributed Maps, AWS StepFunctions has released a few new fields, and deprecated two old ones for Map states. One of them was `Iterator` which was replaced with `ItemProcessor`, the other one is `Parameters` (replaced by `ItemSelector`). A similar PR was made to deprecate `Iterator` (#27913). While they are deprecated, these fields are still supported (PR reflects that) `ItemProcessor` and `ItemSelector` are both fields that are supported in `DISTRIBUTED` and `INLINE` mode, hence why they were added to the existing Map construct Closes #23265 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

… executionType in the ProcessorConfig (#30301) ### Issue # (if applicable) Closes #30194 ### Reason for this change In #27913, the ItemProcessor was introduced for use with the Map Class. With the executionType in the ProcessorConfig, it was possible to specify the executionType for the Map. On the other hand, in #28821, the DistributedMap Class was introduced. The mapExecutionType of the DistributedMap class always overwrites the executionType of the ProcessorConfig. Therefore, when using the DistributedMap class, the implementation ignores the executionType of the ProcessorConfig. However, this behavior cannot be inferred from the documentation. ### Description of changes * Added to the docs that when using the DistributedMap Class, the executionType in the ProcessorConfig is ignored. * Also added a warning. ### Description of how you validated changes Add unit tests and integ tests ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

… executionType in the ProcessorConfig (aws#30301) ### Issue # (if applicable) Closes aws#30194 ### Reason for this change In aws#27913, the ItemProcessor was introduced for use with the Map Class. With the executionType in the ProcessorConfig, it was possible to specify the executionType for the Map. On the other hand, in aws#28821, the DistributedMap Class was introduced. The mapExecutionType of the DistributedMap class always overwrites the executionType of the ProcessorConfig. Therefore, when using the DistributedMap class, the implementation ignores the executionType of the ProcessorConfig. However, this behavior cannot be inferred from the documentation. ### Description of changes * Added to the docs that when using the DistributedMap Class, the executionType in the ProcessorConfig is ignored. * Also added a warning. ### Description of how you validated changes Add unit tests and integ tests ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

feat(stepfunctions): support Map ItemProcessor

25e6375

aws-cdk-automation requested a review from a team November 9, 2023 16:52

github-actions bot added feature-request A feature should be added or improved. p2 distinguished-contributor [Pilot] contributed 50+ PRs to the CDK labels Nov 9, 2023

fixed README

f7af138

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

github-actions bot mentioned this pull request Dec 1, 2023

Monthly PR metrics report - November 2023 #28212

Closed

Merge branch 'main' into awsgh-27878

21e516c

paulhcsun reviewed Dec 11, 2023

View reviewed changes

packages/aws-cdk-lib/aws-stepfunctions/lib/states/state.ts Show resolved Hide resolved

packages/aws-cdk-lib/aws-stepfunctions/lib/states/state.ts Show resolved Hide resolved

kaizencc previously requested changes Dec 13, 2023

View reviewed changes

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 13, 2023

fixed documentation and code style

f4c317a

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 14, 2023

lpizzinidev requested a review from kaizencc December 14, 2023 10:52

kaizencc approved these changes Dec 14, 2023

View reviewed changes

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 14, 2023

Merge branch 'main' into awsgh-27878

1eb9130

mergify bot merged commit ac41730 into aws:main Dec 15, 2023

polamjag mentioned this pull request Jan 4, 2024

(stepfunctions): Add native support for DistributedMap #23216

Closed

2 tasks

abdelnn mentioned this pull request Jan 19, 2024

feat(stepfunctions): support Map ItemSelector #28771

Merged

khushail mentioned this pull request May 16, 2024

aws-stepfunctions: Distributed Map State Item Processor executionType doesn't recognize ProcessorType.EXPRESS #30194

Closed

mazyu36 mentioned this pull request May 22, 2024

fix(stepfunctions): add docs and warning DistributedMap Class ignores executionType in the ProcessorConfig #30301

Merged

1 task

This was referenced Jul 12, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#108

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#109

Open

This was referenced Jul 19, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#110

Open

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#111

Open

NOUIY mentioned this pull request Aug 1, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#112

Open

gitafolabi mentioned this pull request Aug 16, 2024

[Snyk] Upgrade aws-cdk from 2.44.0 to 2.150.0 gitafolabi/kreuzlaker#2

Open

NOUIY mentioned this pull request Aug 20, 2024

[Snyk] Upgrade: , aws-cdk NOUIY/aws-solutions-constructs#113

Open

	private renderProcessorConfig(): any {
	private renderProcessorConfig() {

Conversation

lpizzinidev commented Nov 9, 2023

Uh oh!

Uh oh!

Uh oh!

kaizencc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Dec 14, 2023

Uh oh!

mergify bot commented Dec 14, 2023

Uh oh!

aws-cdk-automation commented Dec 15, 2023

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Dec 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants