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.

Sign up for GitHub

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

Jump to bottom

[Feature]: Support new ASL choice comparators #88

Merged
merged 5 commits into from
Oct 2, 2020
Merged

[Feature]: Support new ASL choice comparators #88

merged 5 commits into from
Oct 2, 2020

Conversation

yoodan93
Copy link
Contributor

@yoodan93 yoodan93 commented Sep 21, 2020 •
edited
Loading

Adding new choice state comparators that were added to the Amazon States Language specifications. https://states-language.net/

These include:

  • StringMatches
  • IsNull, IsPresent, IsString, IsNumeric, IsBoolean, IsTimestamp
  • StringEqualsPath, StringLessThanPath, StringGreaterThanPath, StringLessThanEqualsPath, StringGreaterThanEqualsPath, NumericEqualsPath, NumericLessThanPath, NumericGreaterThanPath, NumericLessThanEqualsPath, NumericGreaterThanEqualsPath, BooleanEqualsPath, TimestampEqualsPath, TimestampLessThanPath, TimestampGreaterThanPath, TimestampLessThanEqualsPath, TimestampGreaterThanEqualsPath

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: StepFunctionsPythonSDK-integtests
  • Commit ID: 16177d7
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: StepFunctionsPythonSDK-integtests
  • Commit ID: 9518733
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

wong-a
wong-a requested changes Sep 24, 2020
View reviewed changes
tests/unit/test_choice_rule.py Outdated
@@ -61,6 +61,35 @@ def test_variable_value_must_be_consistent():
with pytest.raises(ValueError):
func('$.Variable', True)

def test_path_function_value_must_be_consistent():
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def test_path_function_value_must_be_consistent():
def test_path_operator_raises_error_when_value_is_not_a_path():

tests/unit/test_choice_rule.py Outdated
with pytest.raises(ValueError):
func('$.Variable', 'string')

def test_is_function_value_must_be_consistent():
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def test_is_function_value_must_be_consistent():
def test_is_operator_raises_error_when_value_is_not_a_bool():

src/stepfunctions/steps/choice_rule.py Outdated

Args:
variable (str): Path to the variable to compare.
value (bool): Constant value to compare `variable` against.
Copy link
Contributor

Choose a reason for hiding this comment

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

For the type comparisons, maybe the documentation for value can be more descriptive:

  • IsNull: "Whether the value at variable is equal to the JSON literal null or not.
  • IsPresent: "Whether a field at variable exists in the input or not.
  • IsNumeric: "Whether the value at variable is a string or not.
  • etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in subsequent commit

tests/unit/test_choice_rule.py Show resolved Hide resolved
vaib-amz
vaib-amz previously approved these changes Sep 24, 2020
View reviewed changes
rwwardh
rwwardh previously approved these changes Sep 24, 2020
View reviewed changes
Copy link

@rwwardh rwwardh left a comment

Choose a reason for hiding this comment

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

Adam mentioned some naming changes. Once done it should be good to go.

@yoodan93 yoodan93 dismissed stale reviews from rwwardh and vaib-amz via 1f7d4e3 September 25, 2020 04:00
@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: StepFunctionsPythonSDK-integtests
  • Commit ID: 1f7d4e3
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: StepFunctionsPythonSDK-integtests
  • Commit ID: 61fff47
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

wong-a
wong-a requested changes Sep 25, 2020
View reviewed changes
src/stepfunctions/steps/choice_rule.py Outdated Show resolved Hide resolved
src/stepfunctions/steps/choice_rule.py Outdated Show resolved Hide resolved
src/stepfunctions/steps/choice_rule.py Outdated Show resolved Hide resolved
src/stepfunctions/steps/choice_rule.py Outdated Show resolved Hide resolved
src/stepfunctions/steps/choice_rule.py Outdated Show resolved Hide resolved
src/stepfunctions/steps/choice_rule.py Outdated Show resolved Hide resolved
src/stepfunctions/steps/choice_rule.py Outdated

Args:
variable (str): Path to the variable to compare.
value (bool): Constant value to compare `variable` against.
Copy link
Contributor

Choose a reason for hiding this comment

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

I missed this one earlier. The value for StringMatches is a str. Since this is one of the ambiguous operators, let's add a description of the format in the documentation. Customers shouldn't have to go the ASL spec to figure out how to use this SDK.

Suggested change
value (bool): Constant value to compare `variable` against.
value (str): A string pattern that may contain one or more `*` characters to compare the value at `variable` to. The comparison yields true if the variable matches the pattern, where `*` is a wildcard that matches zero or more characters.

Trying to think of good wording about escaping special characters with \. Not sure if there's anything that's handled differently in Python.

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

wong-a
wong-a previously approved these changes Sep 29, 2020
View reviewed changes
src/stepfunctions/steps/choice_rule.py Outdated
Args:
variable (str): Path to the variable to compare.
value (str): A string pattern that may contain one or more `*` characters to compare the value at `variable` to.
The `*` character can be escaped using two backslashes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this docstring will render this correctly if the newlines aren't indented

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: StepFunctionsPythonSDK-integtests
  • Commit ID: 6fd4480
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: StepFunctionsPythonSDK-integtests
  • Commit ID: b5d55f2
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@yoodan93 yoodan93 merged commit 08a0ed7 into aws:master Oct 2, 2020
@wong-a wong-a mentioned this pull request Feb 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vaib-amz vaib-amz vaib-amz approved these changes

@wong-a wong-a wong-a approved these changes

@rwwardh rwwardh rwwardh left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yoodan93 @StepFunctions-Bot @wong-a @vaib-amz @rwwardh