Remove reliance on key order in "do"

[matthias-pichler]

What would you like to be added:

I propose to:

1. only use the continue FlowDirective to continue a loop
2. make then required in all tasks
3. add a start field to the workflow that denotes the first task to execute

document:
  dsl: '1.0.0-alpha1'
  namespace: test
  name: order-pet
  version: '1.0.0'
  title: Order Pet - 1.0.0
  summary: >
  # Order Pet - 1.0.0
  ## Table of Contents
  - [Description](#description)
  - [Requirements](#requirements)
  ## Description
  A sample workflow used to process an hypothetic pet order using the [PetStore API](https://petstore.swagger.io/)
  ## Requirements
  ### Secrets
   - my-oauth2-secret
use:
  authentications:
    petStoreOAuth2:
      oauth2: my-oauth2-secret
  functions:
    getAvailablePets:
      call: openapi
      with:
        document:
          uri: https://petstore.swagger.io/v2/swagger.json
        operation: findByStatus
        parameters:
          status: available
  secrets:
  - my-oauth2-secret
start: getAvailablePets
do:
  getAvailablePets:
    call: getAvailablePets
    output:
      from: "$input + { availablePets: [.[] | select(.category.name == "dog" and (.tags[] | .breed == $input.order.breed))] }"
    then: submitMatchesByMail
  submitMatchesByMail:
    call: http
    with:
      method: post
      endpoint:
        uri: https://fake.smtp.service.com/email/send
        authentication: petStoreOAuth2
      body:
        from: noreply@fake.petstore.com
        to: ${ .order.client.email }
        subject: Candidates for Adoption
        body: >
        Hello ${ .order.client.preferredDisplayName }!

        Following your interest to adopt a dog, here is a list of candidates that you might be interested in:

        ${ .pets | map("-\(.name)") | join("\n") }

        Please do not hesistate to contact us at info@fake.petstore.com if your have questions.

        Hope to hear from you soon!

        ----------------------------------------------------------------------------------------------
        DO NOT REPLY
        ----------------------------------------------------------------------------------------------
    then: end

Why is this needed:

Currently the task execution order is determined by the order in which the tasks appear in the "do" field.
This is very convenient for writing small workflows.
I however see two mayor drawbacks with this approach:

1. similar problems as with a "switch fallthrough". If one forgets a "then" field the execution might resume at a task that is different from the one I expected.

Another interesting case is that the then field is allowed top level in switch tasks. So The way I understand it, this would execute the processElectronicOrder task when the default case is reached because it sets a continue clause.

document:
  dsl: '1.0.0-alpha1'
  namespace: test
  name: sample-workflow
  version: '0.1.0'
do:
  processOrder:
    switch:
      case1:
        when: .orderType == "electronic"
        then: processElectronicOrder
      case2:
        when: .orderType == "physical"
        then: processPhysicalOrder
      default:
        then: continue
  processElectronicOrder:
    execute:
      sequentially:
        validatePayment: {...}
        fulfillOrder: {...}
    then: exit
  processPhysicalOrder:
    execute:
      sequentially:
        checkInventory: {...}
        packItems: {...}
        scheduleShipping: {...}
    then: exit
  handleUnknownOrderType:
    execute:
      sequentially:
        logWarning: {...}
        notifyAdmin: {...}

4. reliance on object keys forces implementers to alway process/store definitions as text blobs or strings.
   Some problems this causes:

• One cannot store the definition as a JSON object in SQL or NoSQL databases as they usually do not preserve Object key order. Thus one cannot use built-in filtering in databases
• When building an API to create workflows the definition has to be accepted as a string or file blob then parsed and validated with the JSON schema and then the parsed object cannot be used because key order is not preserved. This might look something like this in TypeScript:

function parse(input: string): IDefinition {
  const obj: Workflow = JSON.parse(input);

  validate(obj);

  // parse document as map
  const map: Map<"do", Map<string, Task>> = yaml.parse(input, {
    mapAsMap: true,
  });
  // get task names in order of appearance
  const taskNames = Array.from(map.get("do")?.keys() ?? []);
  const tasks = new Map<string, Task>();
  taskNames.forEach((name) => {
    // get task as object instead of Map
    const task = obj.do[name];
    if (task) {
      tasks.set(name, task);
    }
  });

  return { ...obj, do: tasks, text: input };
}

and this only works because Map keys preserve order of insertion, assuming that yaml never changes the implementation that would change the insertion order.

3. when providing a UI to build/visualize workflows the logic to create a valid and correct workflow definition might become incredible complex because on has to be precise about the ordering of keys, handle exit and fallthrough cases etc...

[JBBianchi]

Note that continue might still be useful in loops (e.g.: continue triggers the next iteration, exit exists the loops, end ends the workflow from within the loop).

[matthias-pichler]

Note that continue might still be useful in loops (e.g.: continue triggers the next iteration, exit exists the loops, end ends the workflow from within the loop).

Thanks, good to know 👍 I will updated the initial comment.

On another note: could there be value in renaming exit to break since this more closely aligns to what programmers are already used to? Because I always confuse end and exit and thinking of exiting a process process.exit(0) makes exit even harder to remember correctly.

[JBBianchi]

Note that continue might still be useful in loops (e.g.: continue triggers the next iteration, exit exists the loops, end ends the workflow from within the loop).

Thanks, good to know 👍 I will updated the initial comment.

On another note: could there be value in renaming exit to break since this more closely aligns to what programmers are already used to? Because I always confuse end and exit and thinking of exiting a process process.exit(0) makes exit even harder to remember correctly.

I understand your point but I have 2 remarks:

• supposedly, the DSL should be accessible to non-developers (but, ofc, devs will probably be the main target)
• break kinda makes sense in the context of a loop, but does it really work for a branch ? We went for exit because it felt more natural to "exit" the branch than "breaking" it. It's still very opinionated and subject to discussions.

[matthias-pichler]

Note that continue might still be useful in loops (e.g.: continue triggers the next iteration, exit exists the loops, end ends the workflow from within the loop).

Thanks, good to know 👍 I will updated the initial comment.
On another note: could there be value in renaming exit to break since this more closely aligns to what programmers are already used to? Because I always confuse end and exit and thinking of exiting a process process.exit(0) makes exit even harder to remember correctly.

I understand your point but I have 2 remarks:

• supposedly, the DSL should be accessible to non-developers (but, ofc, devs will probably be the main target)
• break kinda makes sense in the context of a loop, but does it really work for a branch ? We went for exit because it felt more natural to "exit" the branch than "breaking" it. It's still very opinionated and subject to discussions.

I completely understand. I am also unsure about that. Exiting a branch/loop is also how I've come to remember it now.
Interestingly exit and break seem to align 1 to 1 (loops & switch statements).

But yeah lot of overloaded terms in general.

[fjtirado]

I think one of the coolest features of the new DSL is the possibiiltiy to write sequential workflows with very few words. See also this somehow related dicussion
If we remove that reliance in order I feel we were losing a pretty nice feature. Of the three issues, two of them (sorting and UI) are related with implementation difficulties that can be addressed.
This leave us with the the switch, which maybe can be adressed by changing it, so once you start a switch, everything has to be nested inside.

[JBBianchi]

A possibility would be to add a parameter in the document object, like executionSequence with values like strict or loose. Loose would be the default, current behavior, and strict would require to provide a start and mandatory then.

[matthias-pichler]

I think one of the coolest features of the new DSL is the possibiiltiy to write sequential workflows with very few words. See also this somehow related dicussion If we remove that reliance in order I feel we were losing a pretty nice feature.

I agree that for purely sequential workflows this is a very nice feature. However I personally do not think it is worth the footgun. I have two scenarios in mind here:

1. having worked with many workflow languages (Pipedream, Twilio Workflows, AWS Step Functions, Zapier, Cadence, Temporal and having implemented a custom workflow DSL before) the number of times a purely sequential workflow was enough is near zero. (A single API call would already require to handle a 404). In many ways the value of a workflow engine is to declaratively define complex workflows. Some might have started out purely sequentially but very few stayed that way, which leads me to my seconds point:

2. evolving sequential flows:

Assume I have a sequential workflow that doesStuff:

do:
  doStuff1: {}
  doStuff2: {}

when I run into issues where a even number is required I might add a check

do:
  checkParity:
    isOdd:
      when: ${ .x % 2 == 1 }
      then: double
    default:
      then: continue
  double:
    set:
      x: ${ x * 2}
  doStuff1: {}
  doStuff2: {}

I would immediately introduce a bug because x is now always doubled. And I would scratch my head about where I should even place double for it to work.

I believe it would be easier to update a single reference to point to the newly added task than to go around and suddenly have to add then-references everywhere. Having a required then field would allow me to add tasks anywhere in the execution graph by simply changing a single existing then property to point to the new subgraph.

Of the three issues, two of them (sorting and UI) are related with implementation difficulties that can be addressed.

Both the JSON spec as well as the YAML spec define maps/objects as "unordered". So as long as YAML and/or JSON is chosen as the definition language it seems wrong to me to go against these standards.
I believe this makes it incredibly hard for any implementor since basically no existing parsing library can be used. It would require everyone to implement a custom yaml/json parser.

This leave us with the the switch, which maybe can be adressed by changing it, so once you start a switch, everything has to be nested inside.

I think this would be even worse. Because then there would be no way for me to merge multiple branches of a switch task. Then I'd have to either copy and paste all tasks which makes it very verbose and unmaintainable or I'd have to create a completely separate workflow that all branches invoke.

[cdavernas]

I would immediately introduce a bug because x is now always doubled. And I would scratch my head about where I should even place double for it to work.

No, this is not the intended behavior. The next of the switch task is only performed if no default case has been supplied. Your example would therefore work as expected.

[matthias-pichler]

I would immediately introduce a bug because x is now always doubled. And I would scratch my head about where I should even place double for it to work.

No, this is not the intended behavior. The next of the switch task is only performed if no default case has been supplied. Your example would therefore work as expected.

but what about a default with then: continue?

[cdavernas]

Having a required then field would allow me to add tasks anywhere in the execution graph by simply changing a single existing then property to point to the new subgraph.

That is true in theory, but in practice, I never used a serializer implementation which did not preserve the definition order. Plus, that would then be theoretically be true for lists, which should then be indexed thanks to some sort of additional property.

[cdavernas]

but what about a default with then: continue?

Then it proceeds to "double", which is the next task in line.

[matthias-pichler]

Having a required then field would allow me to add tasks anywhere in the execution graph by simply changing a single existing then property to point to the new subgraph.

That is true in theory, but in practice, I never used a serializer implementation which did not preserve the definition order. Plus, that would then be theoretically be true for lists, which should then be indexed thanks to some sort of additional property.

Arrays are defined with order in both specs and parsers respect that.

examples of not preserving order:

JSON.parse

> JSON.parse('{"3+": 1, "2": 2}')
{ '2': 2, '3+': 1 }

https://www.npmjs.com/package/yaml ... also not

[cdavernas]

Arrays are defined with order in both specs and parsers respect that.

I overlooked that.

JSON.parse('{"3+": 1, "2": 2}')

Damn.