Skip to content

Conversation

MEPalma
Copy link
Contributor

@MEPalma MEPalma commented Jan 9, 2025

Motivation

In Step Functions, ItemsPath declarations within Distributed Map state are evaluated only if a JSON input is passed from a previous state in the workflow https://docs.aws.amazon.com/step-functions/latest/dg/input-output-itemspath.html. However, in the current implementation of the SFN v2 interpreter, ItemsPath declarations are always evaluated if declared. This can lead to evaluation failues if the provided path does not exist; AWS instead appears to simply skip the evaluation. These changes evaluate ItemsPath declarations only if this is the only input form for a Distributed Map state, or the Map state is inline. Relevant positive and negative snapshot tests are added.

Changes

  • improve the evaluation logic map state around ItemsPath
  • snapshot tests for the scenarios described

@MEPalma MEPalma added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Jan 9, 2025
@MEPalma MEPalma added this to the 4.1 milestone Jan 9, 2025
@MEPalma MEPalma self-assigned this Jan 9, 2025
Copy link

github-actions bot commented Jan 9, 2025

LocalStack Community integration with Pro

    2 files      2 suites   33m 33s ⏱️
1 370 tests 1 304 ✅ 66 💤 0 ❌
1 372 runs  1 304 ✅ 68 💤 0 ❌

Results for commit c71d919.

Copy link
Member

@joe4dev joe4dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

the level of parity is impressive seeing bugfixes for such special cases :)

self.items_path.eval(env=env)
# ItemsPath in DistributedMap states is only used if a JSONinput is passed from the previous state.
if (
not isinstance(self.iteration_component, DistributedIterationComponent)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sidenote: complexity is with instance check is probably something to keep an eye on in the future if this would become a common pattern, but it's good to learn about such special cases first :)

@MEPalma MEPalma merged commit 630b2a1 into master Jan 9, 2025
36 checks passed
@MEPalma MEPalma deleted the MEP-sfn-fix_items_path_in_distributed branch January 9, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants