-
Notifications
You must be signed in to change notification settings - Fork 159
Backward Compatibility: Parallel Branch does not support direct use of states definitions #413
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
Comments
I can definitely understand your point of view on this.
Can you show me an example of ASL where you find the use of states inside branches useful ? |
@yzhao244 thanks for the example! I was looking for an example where there is actual control-flow logic inside each branches. Off of your example I would actually argue that what we are doing atm is indeed better :) |
@tsurdilo though I mostly agree with your last comment, I think @yzhao244 has a point, too. What if I want to execute two switch cases in parallel? Let's imagine you have a package, and you want to publish it in parallel in both staging and prod. The publishing processes would switch on the package type to determine on which app you actually want to publish it. AFAIK, this cannot be done now without subflows or complex jq expressions. Now, I've had a similar concern with my a client, and we first ended using very complex jq expressions to do so, until we realized it was extremely hard to read and understand with no advanced knowledge of jq and ended up using a sublow, for a single condition check, basically. In my opinion, we should think of a way to do so. Probably not using sub-states, as it looks confusing and goes against the actual state concept, but maybe by adding, for example, a condition property on actions. |
I was not dissageeing with this issue just wanted to find use case and you
provided :)
i wonder if for it it would make more sense to add branch data filter
instead, where you can inject / overwrite state data field for that branch
(one sets prod the other dev or whatever) .
my thinking is that if we allow explicit control flow logic in branches it
will end up creating more issues than bring value eapecially for large
processes (imagine 50+ states in branches and that maintenance)
…On Sat, Jul 3, 2021 at 7:05 AM cdavernas ***@***.***> wrote:
@tsurdilo <https://github.com/tsurdilo> though I agree with your last
comment, I think @yzhao244 <https://github.com/yzhao244> has a point.
What if I want to execute two switch cases in parallel? Let's imagine you
have a package, and you want to publish it in both staging and prod. The
publishing process switches on the package type to determine on which app
you actually want to publish it.
Now, I've had a similar concern with my a client, and we first ended using
very complex jq expressions to do so, until we realized it was extremely
hard to read and understand with no advanced knowledge of jq and ended up
using a sublow, for a single condition check, basically.
In my opinion, we should think of a way to do so. Probably not using
sub-states, as it looks confusing and goes against the actual state
concept, but maybe by adding, for example, a condition property on actions.
@yzhao244 <https://github.com/yzhao244> @tsurdilo
<https://github.com/tsurdilo> WDYT?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#413 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA5E7WG2QHZGWEOM5Y5ZUTTV3VIZANCNFSM47B6WW7Q>
.
|
@tsurdilo @cdavernas Thanks for the replies, guys. :) . Actually, the above AWS DSL example allows UI Console to easily render a graphical model in real-time since it allows end-user directly defines control logic which I believe Google Workflow, Ali Serverless Workflow both allow explicit control flow as well for the purpose of easily rendering graphical model. In our spec, since using subFlowRef in practice requires a workflowId, it requires that end-user has to create subFlow first and then gets a workflowId. Finally, end-user has to put that workflowId in the branches. My personal feeling is a bit complex process and makes UI render graphical model a bit more complicated comparing to allowing explicit control flow. @tsurdilo I looked at your counter-point... it is a very good point of using actions lol.. However, I updated my example with adding retries which our actions do not support retry. :) .. Actually, I totally agree that for better maintenance for large flows, definitely, using subworkflow ID is way better than explicit control flow logic. However, allowing explicit control flow logic in branches shouldn't be a problem because it sounds like a common practice that end-user would want to do. :) Is it possible to support both explicit control flow in branches and subworkflowId which end-user can choose for better maintenance for large flows. |
@tsurdilo @cdavernas For better supporting explicit control flow, I am thinking to set a limit on number of nested layers if end-user chose to define explicit control flow in branches so that our spec can ensure backward compatibility of supporting explicit flow. The following is a suggested example spec of Branch definition for supporting both actions and states definitions. :) Parallel State: Branch
|
they do now (will be hopefully added to 0.7 release :) ) - #435
Overall I do think that supporting both is in the end what we want to do. The way I'd like to do it however is a little different for 3 reasons:
I would like to create a "grouping" of states, with preferably a lable/id attached to them. From the parallel state branch then you can then simply reference this group of states instead of having them hard-coded. WDYT? |
@tsurdilo I read the ActionRetries in 0.7 release. Good Work :) .. |
@yzhao244 yeah, we can/should definitely talk about it to figure it out. Let's plan on that. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Closed as resolved by 1.0.0-alpha1, and therefore as part of #843 In 1.0.0-alpha1, the concepts of You can therefore do the something like: document:
dsl: 0.10
namespace: default
name: amazon-parallel-workflow-equivalency
do:
myParallelState:
execute:
concurrently:
updateMonthlyUsage: { ... }
queueTask:
execute:
sequentially:
invoice: { ... }
email: { ... } |
What is the question:
Hi, I have this issue opened and would like to discuss the spec regarding parallel branch does not support direct use of states definitions when comparing spec 0.1.
If my understanding is correct, currently, the parallel state branch supports actions definition which can use subFlowRef to support a more complex flow such as switch and so on.
However, using subFlowRef in practice requires that end-user has to successfully define a subflow first for getting a workflowId. After obtaining the workflow ID, end-user can start to use the ID to define the parallel state. I think it brings more complexity if end-user only builds a simple parallel workflow which branch only contains a switch, or a operation state and end-user may not need a subFlowRef ID for reusable purpose.
I am sorry to bring back the example of AWS Step function as my reference lol, which allows directly defining states in branch. I feel our specs and theirs have things in common lol..
What would you like to be added:
Is it okay to add back the states array to branch of parallel state definition.
Why is this needed:
The text was updated successfully, but these errors were encountered: