playing with the steno consumer experience with dynamic DAGs

[davepacheco]

(This is very much an exploratory work-in-progress. This may all be a bad idea.)

@ajs -- great call on this. Diving into this made me appreciate the really smart simplicity of the approach. Serializing the DAG itself, binding nodes to actions with string names, and not having to deal with upgrade here -- these make it possible to make a really clean, composeable interface. When I punted on this for so long I was afraid the options were all going to be brittle but this looks solid. And it's good we're doing it now, too. I think this will make it a lot easier (and faster) to develop more complicated sagas. So, thanks for all that!

Some random thoughts

For users of Steno, I think we want to say that any saga can be used as a subsaga. That is, you should be able to drop an existing saga directly into a bigger saga, with a well-defined interface for input and output. The internal details (action names, node names, and anything else) should not matter. I think a consequence of this is that the subsaga's node names need to live in a separate namespace from the parent saga. (Otherwise, it wouldn't be safe to drop some saga into another saga because there might be a conflict of node names, and I really think you shouldn't have to think about that. To me this is just like if you write a function that calls another function -- you don't worry about the local variables in the second function.)

I think a consequence of that is that the DAG needs to be aware that some subgraph is really a subsaga. As we've discussed, it's still one big DAG in terms of execution, but I think there's got to be some way of identifying for a given node which subsaga it was part of. I played around with adding that here (more details below).

Another consideration we talked about is that it should be possible to use subsagas even when you don't know what the subsaga's parameters will be when you're constructing the DAG. The good news is that with the subsaga structure in the DAG, this was pretty easy to do. I think there are some other benefits to this as well.

What's in this change

Action names (so: inputs and outputs) for subsagas are in separate namespaces

• If you're in a subsaga, you cannot see outputs from nodes in the parent saga
• If you're in the parent saga, you cannot see outputs from nodes in a subsaga even if you depend on it
• This applies with arbitrary nesting as well, though I have not tested it
• The subsaga itself has one output, and you can specify the name of it when you add the subsaga to the DagBuilder. So just like you append a Node with a name and the output of the node is associated with that name, you can now add a subsaga with a name and get the output of the whole subsaga by looking up that name.

Saga parameters

Sagas have parameters (like they always have). As with #29, they are not statically typed. But I've re-added ActionContext::saga_params(), so the parameters aren't just the output of an initial node that's added manually. Like lookup(), you have to tell Steno what type to deserialize it as and it'll fail if that doesn't work. Personally I like this because as a consumer you don't need to remember to add that special initial node (or remember what its name is). (This also lets me punt on trying to make this statically type-checked without worrying that it'll be a big breaking change later.)

This change also now supports subsagas where you don't know the saga parameters at the time you're constructing the graph. Instead, when you append the subsaga, you tell Steno which other node's output will become the parameters for the saga. (If you do know the parameters when you're building the DAG, you can create a new "constant" node that just emits them directly (without having to write a function that emits them). I think we should consider adding sugar for this -- a version of append_subsaga that takes the params directly and adds the constant node for you.)

DagBuilder / SagaSpec changes

I've changed things so that:

• a saga or subsaga is always represented as a Dag
• a SagaSpec can now be easily turned into a Dag
• so DagBuilder::append_subsaga now accepts another Dag, and that can be a Dag you created with another DagBuilder or a SagaSpec that's been converted into a Dag

It took me a while to play with this and there's a bit of cleanup to do, but I found this a lot clearer. I'll be interested to see what y'all think. The way I think about it now, you've got Dag + DagBuilder + Node at the lowest layer, and SagaSpec is now basically declarative sugar on top. (Before, these were mingled with sort of overlapping functionality. You couldn't for example add a Dag to another Dag -- you could only add a SagaSpec. So if you wanted to use a builder to make your subsaga, you couldn't.)

instance_id

I removed this but I'm not sure it was for a good reason and we can reconsider this. I don't think it's needed any more now that we're labeling the output of the whole subsaga as a single thing.

Okay but what actually changed

The biggest change is that I turned Node into an enum with variants for Start, End, Action (the thing we used to think of as a node), Constant (which was useful for a few implementation things), SubsagaStart, and SubsagaEnd. It's easiest to see what this looks like by looking at the graphviz rendering of the built-in provision saga:

These nodes contain various bits of information needed to implement the above. For example SubsagaStart includes the name of the ancestor node whose output will be the parameters for this node. SubsagaEnd includes the name that's to be used for the output of the whole subsaga.

(This was all made much easier because of the approach of serializing the whole Dag. It was easy to add different node types with type-specific data.)

I made a bunch of changes to SagaExec::make_ancestor_tree() to implement the node namespacing.

I added SagaExec::saga_params_for() that's used to find the saga parameters for a node (see above), in turn used to implement ActionContext::saga_params().

Plus the changes I mentioned to DagBuilder/SagaSpec.

Okay but what's the user-facing change

From a Steno user's perspective, relative to #29:

• node names for subsagas are in separate namespaces so you never have to keep track of these across different Dags
• each action can get its own saga params in a uniform way
• you don't need to know the saga params for a subsaga when you create the DAG
• you don't need to add the special saga params node explicitly -- you pass that to DagBuilder::new or SagaSpec::to_dag()
• there are no instance ids any more (but this may need to be revisited)

What's still to do

I took a lot of really nasty shortcuts in SagaExec to see what this would look like. If y'all think it's good, these shouldn't be too bad to clean up. But right now they're really ugly.

I'm interested to see if we can both use newtypes for some of the string fields (like NodeName and NodeLabel?) and also changing the functions that accept them to use AsRef<str> so that we have stronger safety once they're constructed but users don't have to use all the explicit constructors. What do y'all think?

I had some other comments on #29 that I'm also interested to see what y'all think of.

[andrewjstone]

@davepacheco This is freakin amazing. I love how easy and mostly straightforward it was to add all the missing functionality I wanted in #29. I'm very happy to be rid of instance ids. There's no longer a need for them with these changes.

Thank you so much for the detailed writeup on the PR. That was a great explanation. I don't disagree with anything said and the changes all make sense!

I left a few comments, but this works, so I'm merging into #29 and we'll continue iterating there.

[andrewjstone]

I'm very happy to get rid of this comment and the possibility of create_params not existing at all.

[andrewjstone]

I really like that this now takes a Dag and the SagaSpec is completely a separate layer that just maps to the Dag.

[andrewjstone]

It's not that dirty 😆

[andrewjstone]

Ok, I'm a touch scared here 😆