Skip to content

Conversation

@andydouglas-exs
Copy link
Contributor

Previously using the following syntax: "ParamI": StateTwo.output() failed with

ValueError: State 'StateThree' is using an illegal placeholder for the 'ParamI' parameter.

I've added a simple fix and extended the existing test cases so that StepInput placeholders behave similarly to ExecutionInput

AD

@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: FAILED
  • Build Logs (available for 30 days)

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

if isinstance(v, StepInput) and v.name and not step_output.contains(v):
return False, k
if isinstance(v, StepInput):
if v.name and not step_output.contains(v):
Copy link
Contributor

Choose a reason for hiding this comment

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

The proper fix should not be checking the v.name but to verify if v is the step_output.

For instance, if this is the input.

{
    "ParamI": StepTwo.output()
}

This will make not step_output.contains(v) to be False because StepTwo.output().contains(StepTwo.output()) == False. Because we don't want to fail the validation, we only need to check if the output itself is being passed.

elif v.contains(placeholder):
return True
return False
return self.contains(placeholder)
Copy link
Contributor

Choose a reason for hiding this comment

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

You should keep the old logic here, and add a new method called __contains__ so that we can make v not in step_output work.

    def __contains__(self, placeholder):
        return self.contains(placeholder)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies, I missed the dunder I've reinstated and added a new method

@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

@shunjd shunjd self-requested a review November 21, 2019 00:28
@shunjd shunjd merged commit 662ead6 into aws:master Nov 21, 2019
andydouglas-exs added a commit to andydouglas-exs/aws-step-functions-data-science-sdk-python that referenced this pull request Nov 25, 2019
StepInput placeholder without key should resolve to $ (aws#7)
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.

4 participants