Skip to content
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

[SFN] Handling of Unsupported ASL Bindings #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lizard-boy
Copy link

Motivation

Currently the SFN v2 interpreter fails on parsing whenever a unknown production is added to the derivation. This leads to the user being unable to run the entire state machine if at least one derivation is unsupported. The most recent instance of is the use of Label fields (currently unsupported) in Map states localstack#11029, however this occurred in other instances such as Version, and Tolerance values. Although this project aims to support all ASL features, until a feature is supported, the user is unable to create the state machine, with the only solution being the manual removal of the unsupported production.
This PR wants to allow the user to create and execute the state machine regardless of unsupported language features, whilst being warned of the incident.

Changes

  • add skip production for unknown bindings for top layer and state level statements
  • add relevant test case

TODO

What's left to do:

  • Determine whether to enable this feature only when "Strict mode" is turned off (through an environment variable)

@lizard-boy lizard-boy changed the title support for unknown bindings [SFN] Handling of Unsupported ASL Bindings Sep 20, 2024
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added support for unknown bindings in the Step Functions ASL parser, allowing state machines with unsupported features to be created and executed while warning users of their presence.

  • Introduced 'unknown_binding_decl' rule in ASLParser.g4 for top-level and state-level statements
  • Added 'visitUnknown_binding_decl' method in preprocessor.py to log warnings for unknown bindings
  • Created new test case 'test_unknown_bindings' in test_base.py to validate handling of unsupported features
  • Updated test snapshots and validation files to include the new test case for unknown bindings

7 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings

Comment on lines +411 to +412
template["UnknownBinding"] = "StringValue"
template["States"]["State_1"]["UnknownBinding"] = {"KeyValue": [], "End": {}}
Copy link

Choose a reason for hiding this comment

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

style: consider using a more descriptive key name than 'UnknownBinding' to clarify its purpose

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants