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.

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

Bug in a workflow with 2 path to the same app #40

Open
tristandostaler opened this issue Jan 24, 2022 · 5 comments
Open

Bug in a workflow with 2 path to the same app #40

tristandostaler opened this issue Jan 24, 2022 · 5 comments

Comments

@tristandostaler
Copy link

I have the following workflow with 2 path possible to the same node:
Option 1: if $exec.fields contains computerName, do node A, then do node B
Option 2: if $exec.fields does not contains computerName, do node B.

However, when testing with a case where the Option 1 should be followed, the node B is never executed because the condition for the Option 2 is not fulfilled (that's the reason given when looking at the output of the skipped node B).

image

@frikky
Copy link
Member

frikky commented Jan 25, 2022

I have the following workflow with 2 path possible to the same node: Option 1: if $exec.fields contains computerName, do node A, then do node B Option 2: if $exec.fields does not contains computerName, do node B.

However, when testing with a case where the Option 1 should be followed, the node B is never executed because the condition for the Option 2 is not fulfilled (that's the reason given when looking at the output of the skipped node B).

image

Hola,

I think you're right! It should work like this. Right now, it works with AND rather than OR between them, meaning everything has to match going into the same node. The reason it hasn't been explored enough is probably because we've done what's seen below all along instead. The piece of code that handles this is also in the app itself, and can be seen here: https://github.com/frikky/Shuffle/blob/7a388f3c055baac37cc0a20ba583f4380f8f1561/backend/app_sdk/app_base.py#L1996

Do you think the best way to handle it may be to see if just one matches? It would just be a counter, checking if it's more than 0 at that point. It may also break someone elses workflow, but I totally do agree with this one.

image

@tristandostaler
Copy link
Author

Hum ok the thinking is different than what I had in mind. But I think it translates to a branch should be an OR. So as soon as any branch goes to a node, the node executes. It would work in both examples we have in this thread. With your examples it works out of the box, and for my example I don't have to add an empty node between the router and the node B

@tristandostaler
Copy link
Author

The way to handle "the node executes if all branches are true" would be through conditions in the output of the previous nodes.

@tristandostaler
Copy link
Author

tristandostaler commented Jan 25, 2022

Btw, what I have in mind when I see this type of workflow is a "pipe and filter" architectural pattern (documentation bellow).
In a pipe and filter pattern, there is no "if logic" on a pipe (a branch between 2 nodes) so here there is a small nuance. But for me, it's an "if" that stand before (or after) the pipe logically, and the pipe is simply a link between 2 objects.
In your context, a pipe would be an object which link 2 nodes toghether and can have an array of conditions, but when executing the workflow, I would follow the pipe and filter pattern and evaluate the if just before or just after sending data to the following node.

https://www.dossier-andreas.net/software_architecture/pipe_and_filter.html
https://docs.microsoft.com/en-us/azure/architecture/patterns/pipes-and-filters
https://student.cs.uwaterloo.ca/~cs446/1171/Arch_Design_Activity/PipeFilter.pdf

@frikky
Copy link
Member

frikky commented Jan 26, 2022

Hum ok the thinking is different than what I had in mind. But I think it translates to a branch should be an OR. So as soon as any branch goes to a node, the node executes. It would work in both examples we have in this thread. With your examples it works out of the box, and for my example I don't have to add an empty node between the router and the node B

Small clarification: it will ALWAYS wait for all previous nodes to have done something before executing, even when just one has to be correct. This is to ensure that we're taking all results into account, and it's not running the workflow in the wrong order

This should be working when we update our apps to the latest SDK which has a proper fix for it :)

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

No branches or pull requests

2 participants