feat(stepfunctions): retrieve all reachable states from a given state in a state machine definition #7324

andrestone · 2020-04-12T20:35:36Z

Commit Message

feat(stepfunctions): retrieve all reachable states from a given state in a state machine definition (#7324)

A new method - findReachableStates - to retrieve all reachable states subsequent to a given initial state, including itself.

Motivation
With this method, developers can programatically modify their state definitions, given an initial state.
For example, walk through the set of states and attach some of them to a 'ResumeTo' Choice state, so that state machine executions can jump directly to one of these states.

closes #7256

End Commit Message

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation · 2020-04-12T21:09:05Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 4de59c8
Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-12T21:20:10Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: cfe7b60
Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-12T22:23:04Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3b066c6
Result: FAILED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-12T23:00:46Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3b82f72
Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-15T15:28:28Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 27db11a
Result: FAILED
Build Logs (available for 30 days)

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

andrestone · 2020-04-15T15:34:19Z

Updated tests because of #7351.

aws-cdk-automation · 2020-04-15T15:58:19Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 29639cd
Result: SUCCEEDED
Build Logs (available for 30 days)

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

nija-at

Thanks for the contribution! 😊

Could you add your motivation or use case (to the commit message) on why you're adding this method and how you plan to use it?

andrestone · 2020-04-16T11:41:06Z

Thanks for the contribution! 😊

Could you add your motivation or use case (to the commit message) on why you're adding this method and how you plan to use it?

Hi! Thanks for reviewing.

I tried to be more explicit now. Please let me know if more information is needed.

nija-at · 2020-04-16T12:48:16Z

Hi @andrestone - thanks for updating. I'm not sure I quite follow what you've said. Can you expand or provide a concrete example?

andrestone · 2020-04-16T13:04:17Z

Hi @andrestone - thanks for updating. I'm not sure I quite follow what you've said. Can you expand or provide a concrete example?

Sure.

This method takes an original definition and adds a ResumeTo Choice state to the top of the chain, programatically adding each existing state id as a Choice condition to be matched.

private static transformToResumable(definition: sf.IChainable): sf.IChainable {
    // Building a set of States
    const states = new Set<sf.State>();

    // All states starting from a giving state (using the method of this PR)
    sf.State.findReachableStates(definition.startState).forEach(state => {
        states.add(state);
    });

    // Here we add our ResumeTo logic to the top of the chain
    const choices = new sf.Choice(scope, "ResumeTo");

    // We used Set to ensure we have only a single state, although it's redundant with
    // the method implementation
    let statesArray = Array.from(states);

    // For each pre-existing state, we add a Choice condition
    for (let i = 0; i < statesArray.length; i++) {
        choices.when(sf.Condition.stringEquals('$.resumeTo', statesArray[i].id), statesArray[i]);
    }

    // The default is the original definition
    choices.otherwise(definition);

    // This is our new State Machine definition ready to accept `resumeTo` as part of the input
    // to resume the execution starting from the referred state
    return choices
}

Should I add this to the commit message?

Thanks.

andrestone · 2020-04-16T13:21:31Z

I tried to rewrite the description, it was indeed confusing.

Not sure if it's clear enough now, though.

Thanks for reviewing.

nija-at · 2020-04-16T13:52:19Z

Thanks! This does sound interesting. I've re-worded the commit message as per this. Feel free to adjust it if I was inaccurate.

andrestone · 2020-04-16T14:09:39Z

Thanks! This does sound interesting. I've re-worded the commit message as per this. Feel free to adjust it if I was inaccurate.

It's definitely better, but I think it's important to mention we're dealing with the State Machine definition, not the State Machine itself. Just for clarity.

nija-at · 2020-04-16T14:11:13Z

packages/@aws-cdk/aws-stepfunctions/README.md

+To retrieve all possible subsequent states from an initial state, you can use the
+method `State.findReachableStates`:
+
+```typescript
+        import * as sf from '@aws-cdk/aws-stepfunctions';
+        const nextState = sf.State.findReachableStates(startState); // All reachable / next states
+```
+


I don't think you need the code block for this since the API is straightforward. How about changing it to something like this -

Suggested change

To retrieve all possible subsequent states from an initial state, you can use the

method `State.findReachableStates`:

```typescript

import * as sf from '@aws-cdk/aws-stepfunctions';

const nextState = sf.State.findReachableStates(startState); // All reachable / next states

```

A few utility functions are available to parse state machine fragments.

* `State.findReachableStates`: Retrieve the list of states reachable from a given state.

* `State.findReachableEndStates`: Retrieve the list of end or terminal states reachable from a given state.

nija-at · 2020-04-16T14:13:59Z

packages/@aws-cdk/aws-stepfunctions/test/states-language.test.ts

      .branch(state2)).toThrow();
+  }),
+
+  test('Can retrieve possible states from initial state', () => {


Need more test cases here -

Assert that this behaves correctly when there is a Choice and Parallel states included.

Assert that states that are not reachable are not returned.

Wrap these test cases inside of a describe clause -

describe('findReachableStates', () => { test('Can retrieve possible states from initial state', () => { ... }); });

The construct sees the Parallel branches as separate State Machines, so none of the private methods used to compute the subsequent states actually care about them. It looks to me like it's a design decision that is out of the scope of this PR.
Therefore, the states inside a Parallel states are not retrieved.

I'm not so sure that this is a conscious design choice.

It seems to me that outgoingTransitions() should take an additional option called includeBranches (which defaults to false) and considers all the branches (this.state) of the current state as well.

Otherwise, this utility function is incomplete.

cc @rix0rrr.

Parallel branches are separate state machines, as far as I'm aware, so yes this was done consciously. Let me read the rest of the thread.

I'm not so sure that this is a conscious design choice.

It seems to me that outgoingTransitions() should take an additional option called includeBranches (which defaults to false) and considers all the branches (this.state) of the current state as well.

Otherwise, this utility function is incomplete.

cc @rix0rrr.

I like this approach better. Please let me know if I should add it in this PR.

Given that we don't need this yet, let's leave the implementation as is, and add explicit documentation to the tsdoc that these states are not returned.

andrestone · 2020-04-16T16:28:43Z

I accidentally force pushed a wrong commit. Trying to make this PR survive the mess, ~~not sure I can make it :)~~

Pull request has been modified.

aws-cdk-automation · 2020-04-16T16:50:22Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 4ab3ffa
Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-16T17:07:23Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8334ebe
Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-16T18:39:48Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 6f2692e
Result: SUCCEEDED
Build Logs (available for 30 days)

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

rix0rrr · 2020-04-20T11:54:27Z

I guess I should put more effort into understanding the purpose behind this PR, but I'm short on time so I'll just throw this out:

With this method, developers can programatically modify their state definitions, given an initial state.
For example, walk through the set of states and attach some of them to a 'ResumeTo' Choice state, so that state machine executions can jump directly to one of these states.

Chain.next() already can do this, so if you have a Chain object with your subgraph in it, you can already do what you want. You would normally obtain such a Chain object by building it using the fluent API, but I guess you are setting up your app slightly differently. Could you share some of your state machine definition code? Maybe there's something even more simple/powerful we could be doing to make it easier for you to achieve the goal you're trying to get to.

If we keep roughly to the current approach, given that we want a Chain with your desired end states in it, the problem then becomes how to go from a starting state to a Chain object. Maybe you can add a Chain.discoverFromStartingState(): Chain method to cover this functionality.

I will admit the API between IChainable, INextable, Chain and State is currently a little silly. It's on the docket to be fixed up.

andrestone · 2020-04-20T12:32:16Z

I guess I should put more effort into understanding the purpose behind this PR, but I'm short on time so I'll just throw this out:

With this method, developers can programatically modify their state definitions, given an initial state.
For example, walk through the set of states and attach some of them to a 'ResumeTo' Choice state, so that state machine executions can jump directly to one of these states.

Chain.next() already can do this, so if you have a Chain object with your subgraph in it, you can already do what you want. You would normally obtain such a Chain object by building it using the fluent API, but I guess you are setting up your app slightly differently. Could you share some of your state machine definition code? Maybe there's something even more simple/powerful we could be doing to make it easier for you to achieve the goal you're trying to get to.

Hi, @rix0rrr, thanks for your interest.

As per what I can understand, the Chain.next() method has the sole purpose of building a state machine definition by "chaining" Chain objects together, binding those by reference in these properties: startState, endStates and id.

What I'm trying to accomplish here, is to programatically modify (most likely not in place, such as demonstrated in the example above) a state machine definition somewhere out of the scope of its original declaration.

In order to do that, I need to have access to those chained objects. As mentioned in the original issue, I couldn't find anyway to do it, hence this PR.

If we keep roughly to the current approach, given that we want a Chain with your desired end states in it, the problem then becomes how to go from a starting state to a Chain object. Maybe you can add a Chain.discoverFromStartingState(): Chain method to cover this functionality.

I will admit the API between IChainable, INextable, Chain and State is currently a little silly. It's on the docket to be fixed up.

I think it would also make sense to have the method as part of the Chain class, although State seems to be a closer abstraction to what we're trying to address. And since there was already a method doing something very close to what I needed inside the State class, I thought it would be more appropriate to have this new one there.

packages/@aws-cdk/aws-stepfunctions/test/states-language.test.ts

packages/@aws-cdk/aws-stepfunctions/lib/states/state.ts

packages/@aws-cdk/aws-stepfunctions/test/states-language.test.ts

nija-at · 2020-04-21T13:14:20Z

packages/@aws-cdk/aws-stepfunctions/test/states-language.test.ts

      .branch(state2)).toThrow();
+  }),
+
+  test('Can retrieve possible states from initial state', () => {


Given that we don't need this yet, let's leave the implementation as is, and add explicit documentation to the tsdoc that these states are not returned.

nija-at

Thanks for making the changes. A few more minor tweaks and this should be ready to go.

Pull request has been modified.

… in a state machine definition

andrestone · 2020-04-21T14:44:53Z

Done!

Only extra is that I switched the assertions to .id, the toStateJson() was giving me false positives.

Thanks, @nija-at.

aws-cdk-automation · 2020-04-21T14:48:21Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: c2a5bf1
Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-21T14:48:34Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3637791
Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-21T14:48:48Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: c2a5bf1
Result: SUCCEEDED
Build Logs (available for 30 days)

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

aws-cdk-automation · 2020-04-21T15:17:43Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 64a6826
Result: SUCCEEDED
Build Logs (available for 30 days)

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

nija-at

Thanks for making those changes. Approved!

Our mergify bot will take it from here and merge this PR into our master branch.

mergify · 2020-04-22T09:41:07Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2020-04-22T10:29:33Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 2f724db
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2020-04-22T10:29:53Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

andrestone force-pushed the andrestone/sfn-reachable-states branch from 4de59c8 to cfe7b60 Compare April 12, 2020 20:46

andrestone force-pushed the andrestone/sfn-reachable-states branch from cfe7b60 to 3b066c6 Compare April 12, 2020 22:20

andrestone force-pushed the andrestone/sfn-reachable-states branch from 3b066c6 to 3b82f72 Compare April 12, 2020 22:26

SomayaB assigned nija-at Apr 14, 2020

andrestone force-pushed the andrestone/sfn-reachable-states branch 2 times, most recently from 27db11a to 29639cd Compare April 15, 2020 15:24

nija-at suggested changes Apr 16, 2020

View reviewed changes

nija-at changed the title ~~feat(aws-stepfunctions): a method to retrieve all possible next states from a given initial state~~ feat(stepfunctions): retrieve all reachable states from a given initial state for a state machine Apr 16, 2020

nija-at mentioned this pull request Apr 16, 2020

RFC(StepFunctions): A way to have States inside a Chain more easily exposed #7256

Closed

nija-at changed the title ~~feat(stepfunctions): retrieve all reachable states from a given initial state for a state machine~~ feat(stepfunctions): retrieve all reachable states from a given state in a state machine Apr 16, 2020

nija-at previously requested changes Apr 16, 2020

View reviewed changes

nija-at changed the title ~~feat(stepfunctions): retrieve all reachable states from a given state in a state machine~~ feat(stepfunctions): retrieve all reachable states from a given state in a state machine definition Apr 16, 2020

andrestone closed this Apr 16, 2020

andrestone force-pushed the andrestone/sfn-reachable-states branch from 29639cd to 4ab3ffa Compare April 16, 2020 16:17

andrestone reopened this Apr 16, 2020

andrestone requested a review from nija-at April 18, 2020 13:52

andrestone requested a review from rix0rrr April 20, 2020 15:34

nija-at reviewed Apr 21, 2020

View reviewed changes

nija-at previously requested changes Apr 21, 2020

View reviewed changes

feat(stepfunctions): retrieve all reachable states from a given state…

64a6826

… in a state machine definition

andrestone force-pushed the andrestone/sfn-reachable-states branch from c2a5bf1 to 64a6826 Compare April 21, 2020 14:43

nija-at approved these changes Apr 22, 2020

View reviewed changes

Merge branch 'master' into andrestone/sfn-reachable-states

2f724db

mergify bot merged commit ac3b330 into aws:master Apr 22, 2020

feat(stepfunctions): retrieve all reachable states from a given state in a state machine definition #7324

feat(stepfunctions): retrieve all reachable states from a given state in a state machine definition #7324

Uh oh!

Conversation

andrestone commented Apr 12, 2020 • edited by nija-at Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Commit Message

End Commit Message

Uh oh!

aws-cdk-automation commented Apr 12, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 12, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 12, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 12, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 15, 2020

AWS CodeBuild CI Report

Uh oh!

andrestone commented Apr 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aws-cdk-automation commented Apr 15, 2020

AWS CodeBuild CI Report

Uh oh!

nija-at left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrestone commented Apr 16, 2020

Uh oh!

nija-at commented Apr 16, 2020

Uh oh!

andrestone commented Apr 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrestone commented Apr 16, 2020

Uh oh!

nija-at commented Apr 16, 2020

Uh oh!

andrestone commented Apr 16, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrestone Apr 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrestone Apr 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrestone commented Apr 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aws-cdk-automation commented Apr 16, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 16, 2020

AWS CodeBuild CI Report

Uh oh!

aws-cdk-automation commented Apr 16, 2020

AWS CodeBuild CI Report

andrestone commented Apr 12, 2020 •

edited by nija-at

Loading

andrestone commented Apr 15, 2020 •

edited

Loading

nija-at left a comment •

edited

Loading

andrestone commented Apr 16, 2020 •

edited

Loading

andrestone Apr 16, 2020 •

edited

Loading

andrestone Apr 16, 2020 •

edited

Loading

andrestone commented Apr 16, 2020 •

edited

Loading

rix0rrr commented Apr 20, 2020 •

edited

Loading

andrestone commented Apr 20, 2020 •

edited

Loading

andrestone commented Apr 21, 2020 •

edited

Loading