fix(stepfunctions): add docs and warning DistributedMap Class ignores executionType in the ProcessorConfig#30301
Merged
mergify[bot] merged 8 commits intoaws:mainfrom Jun 13, 2024
Conversation
mazyu36
commented
May 22, 2024
| */ | ||
| public toStateJson(): object { | ||
| let rendered: any = super.toStateJson(); | ||
| if (this.mapExecutionType) { |
Contributor
Author
There was a problem hiding this comment.
Removed this conditional statement as it is meaningless, because in the following code, mapExecutionType is always set, making the condition always evaluate to true.
gracelu0
previously requested changes
May 31, 2024
Contributor
gracelu0
left a comment
There was a problem hiding this comment.
Hi @mazyu36 , thanks for working on this! Could you add a note in the docstring for the mapExecutionType property and update the README file as well so users know they should use that property to set the execution type for ItemProcessor. Thanks!
Co-authored-by: Grace Luo <54298030+gracelu0@users.noreply.github.com>
Contributor
Author
|
@gracelu0 |
gracelu0
reviewed
Jun 3, 2024
| distributedMap.itemProcessor(new sfn.Pass(this, 'Pass State')); | ||
| ``` | ||
|
|
||
| If you want to specify the execution type for the DistributedMap, you must set the `mapExecutionType` property in the `DistributedMap` class. When using the `DistributedMap` class, the `ProcessorConfig.executionType` property is ignored. |
Contributor
There was a problem hiding this comment.
Do we mean 'execution type for ItemProcessor'? I don't see a mapExecutionType property in the docs
Contributor
Author
There was a problem hiding this comment.
Thank you for the review.
As you correctly understood, I have revised the expression to 'ItemProcessor in the DistributedMap' for more accurate phrasing.
|
|
||
| If you want to specify the execution type for the DistributedMap, you must set the `mapExecutionType` property in the `DistributedMap` class. When using the `DistributedMap` class, the `ProcessorConfig.executionType` property is ignored. | ||
|
|
||
| In the following example, the execution type for the DistributedMap is set to `EXPRESS` based on the value specified for `mapExecutionType`. |
Contributor
There was a problem hiding this comment.
Same as above, itemProcessor instead of DistributedMap?
Contributor
Author
There was a problem hiding this comment.
I have made the same revision as the one above.
d631069 to
c5b8b0c
Compare
Contributor
|
@Mergifyio update |
Contributor
✅ Branch has been successfully updated |
gracelu0
approved these changes
Jun 13, 2024
Contributor
gracelu0
left a comment
There was a problem hiding this comment.
Thank you for contributing!
Collaborator
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Contributor
|
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). |
sarangarav
pushed a commit
to sarangarav/aws-cdk
that referenced
this pull request
Jun 21, 2024
… 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*
mazyu36
added a commit
to mazyu36/aws-cdk
that referenced
this pull request
Jun 22, 2024
… 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*
This was referenced Jul 12, 2024
This was referenced Jul 19, 2024
Collaborator
|
Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Description of how you validated changes
Add unit tests and integ tests
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license