-
Notifications
You must be signed in to change notification settings - Fork 153
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
[workflow] Terminology, Scope and Specification Core simplification #129
Comments
@salaboy thanks for writing this up! Here are some things to consider I believe:
Regarding the "simplification section" of the writeup:
Regarding the "Data Flow" section -- I think this is where I am failing to understand the reasoning behind the proposal the most. This specification is trying to target both stateless and stateful serverless orchestration scenarios. What I wanted to ask you, "Event producer" was mentioned to be one of the core tasks to consider, clearly a task that produces events that include "data", how is it then that workflow data is considered an extension point itself. Please don't take my comments as all negative, I think there is alot of things to learn from this especially that we do need to simplify things it seems that are currently present. Also a big takeaway from me is that there is still a ton of work it seems on making sure everything is clearly defined and a we need to add alot more examples and use cases, as well as comparisons and conversions from other workflow json formats. I am a little weary however to consider this specification "too hard to understand" or "too complex". I believe that the people saying that do have the capabilities to voice that themselves via issues or github request, and really urge those people to speak up and give us their opinions as they are so valuable. If however the same people just look at what we currently have and don't voice out their opinion and contribute, from my experience they are not going to use this spec even if we went ahead and did a complete change. |
@tsurdilo thanks a lot for taking the time for writing such a detailed answer. 1 - State Machine VS Workflow (Terminology & Scope): Regarding my proposal to rename:
I purposely try to keep it one to one, but at the same time, I wanted to highlight the constructs that we should focus on for the first iteration of the spec. I have the feeling that we should focus on these core concepts and leave all other possibilities out for now. WDYT? 2 - “A Task represents a unit of work at runtime” Summing up, my proposal here would be to have:
Here, the Workflow act as a controller to all its Tasks and we promote using SubFlows (hierarchy) to build higher-level models, only if needed (and only if needed is the important part). 3 - Simplification 4 - DataFlow Scope
I think that you can notice from this proposal that I am trying to making things simple, by doing this I am aware that we will not cover all use cases, but we will make it simple to use and to understand. Notice as well, how we don’t need strict binding between inputs/outputs from each Task, and we can just give hints to the implementations on how to produce such information based on metadata/headers. I wonder how conceptually faraway are we on this point? What are the compromises that we can make to keep it simple and usable? Issues / PRsI propose to have an issue for each of these topics so people interested in just one section can comment on each of these buckets. This can help us also to lead changes if we consider that changes are worth after discussing them. Do these issues make sense:
I've tried to summarize the feedback that I got in KubeCon San Diego and also trying to help people to raise their voices by adding these long threads. I don't think that people will comment unless we clearly explain where we are and where we are going. |
I think the current specification is still far away from being optimal, especially on the writeup part, and we lack more examples etc, but I do believe it is
Of course I agree with you that it should be as simple as possible, so yeah we should invest in your proposal of a "core" vs "extended" concept, but on the other side we have super complex things like bpmn2 which deal w/ that via the conformance definition. Which one is better idk. There is no strict binding currently, the output of a task is passed as input to next as json object. we use json path for data filtering. I believe this has even the advantage over things like bpmn2 where for example alot more strict input/output bindings have to be defined. Sorry last thing regarding the mentioned possible confusion of like task, operation task, action, function etc. My opinion on this is that we have to see why people would even use a serverless workflow, this is described in the intro section currently. If I wanted to execute 10 functions in a sequence i would never use a workflow solution for that to begin with. The actual function call is encapsulated inside an action currently because the results of a function might have to be filtered and checked for validity/errors in order to achieve correct business logic. In addition an action may do more than just call a serverless function - business logic might require utilizing existing integration services that may or may not be "serverless" in some cases. So I think we should try to via extensibility as you mentioned cover those cases as well. We can try to lower the complexity of the json or try not to have too much nesting, but having a task thats only responsible to call a function imo defeats the purpose of a workflow solution (when I can write that as a 1-liner without workflow). wdyt? Regarding the overall data flow discussion. If you look at the current specification, it is completely optional already. The workflow data input/output is optional. Different types of filters are again optional. You dont have to use the workflow data to pass as parameters to function calls. Thanks again, and really nice conversation! |
@tsurdilo once again thanks for the detailed answer, here my comments and action list.
I encourage community people to get in touch if they have comments about these topics. For that reason, I would like to keep this issue open for a while until we decide that we have fixed some of these problems and we are all in the same page. |
This is an interesting talk, which will help our discussion on this, I think. |
Task is a concrete piece of work to be done. State is a stage in the workflow and coveys more info than a task, for example, you can define trigger event, how to do retry, how information is passed, in addition to the task. Some people think a serverless workflow/graph can be naturally modeled as a state machine. Disregard that, a state is not necessarily associated with a finite state machine, imo. I think using task will be more confusing. Replacing event state with event consumer will cause more confusion since in CloudEvents, it is stated that events might also be routed to consumers, like embedded devices... People can argue that the workflow system is an event consumer or the serverless function is the event consumer or switch state is also an event consumer. I agree with most of Tihomir's comments. |
I don't see the point of keep discussing at this very abstract level without concrete examples -- agreed, that's why I added more examples and will keep adding more of them. That's the best way I think to see what may be confusing or needs better explanation. This is where it gets confusing.. if we have a workflow context we don't really need to move data from the output to one task to the input of the next one. I would be in favor of just having a workflow context. WDYT? -- I am honestly not in favor of this. It may work for small workflows but for anything production-ready having a single global context holding on data is not ideal and imo will add to the confusion rather than help. We might look at having something similar to bpmn2 where you have process "global" data and subprocess/state/task "private data". Regarding context specifically, there is a difference between "workflow context" and "execution context". I think what we are talking about here is the "workflow context" which can hold "global data" each state can have access to. For stateful orchestration implementations can define an execution context which can hold workflow data that should be persisted, it may include the workflow context (prob should :) ). This is something that we can definitely define imo. Also agree that we need concrete things (via examples please -- spec-examples.md) that can be used to see where we can make our improvements. Same thing please for describing why some features are confusing, as I still don't see how this spec is confusing at all. It is fairly small and compared to bpmn2, and aws even it is much much simpler to use and understand imo. |
Also just wanted to add - I think documentation will be much cleaner when serverless workflow spec gets its own github repo and we can do a proper wiki..and maybe one day we get a site like https://cloudevents.io :) |
|
@cathyhongzhang thanks for your input.. very good talk about AWS Step Functions, do you think that Step is more clear than Task? |
@cathyhongzhang can you help us to get a separate repo for the group? can we open another issue for that? so we know if we are making progress around that or not? |
@salaboy Yes, I think step is more clean than task. Or we can use stage as suggested by Ruben. |
@salaboy I am not sure if I have the permission to get a separate repo. I will check the Doug on that. |
@salaboy after thinking about this for a while I have to say that unlike the "popular opinion" that we should keep "state" and not use your proposed "task":
As far as "stage" goes, definitions says "a point, period, or step in a process or development" - this does not sound right to me as again it is somewhat related to a time and is to me more related to entities like users then a "thing" like a workflow. I think just saying "ParallelTask" or "ParallelStage" ... task sounds much better imo :) So @ALL I think we should cast a vote for this maybe, @salaboy could you create a poll or something for it? Or is there maybe a better idea on how to get everyones vote in. |
@tsurdilo I am happy with a poll... I've just sent a request to install an app for a Poll into GitHub issues to the admin of the repo. |
I think step (as used by AWS) or stage as suggested by Ruben is better than task. Task is a piece of work that needs to be done. The "state" entities we define in the workflow denote more meanings than a task. If the terminology "state" is not the optimal, then "step" or "stage" is a better name. Let's discuss this in the weekly meeting to get input from more people. |
@salaboy Whom have you sent the request for getting a separate repo? Actually Doug already replied to me saying that CNCF did not approve a separate repo for workflow. |
Here is my 2 cents: I definitely don't like using the state machine and state to represent the workflow as I mentioned at #127 (comment). I think just replacing state machine with workflow, and state with step will make things easy to understand. Another datapoint is that this spec is not limited to stateful orchestration, what does the "state" mean if it's a stateless workflow? Just like how Step Functions defines its state, Step is an abstraction and can have different concrete steps, such as task step, parallel step, foreach step... Regarding stage, I feel it's a bit similar to state but without using the state word (which is good). When we model a workflow, do we use verbs or adjectives, if the answer is verb, is step better than stage? Also personally I use it less often than step, e.g. execute the workflow step by step (not stage by stage). |
@wanghq I totally agree with your comment, |
on a side note :) just had one of those "holy @#!& im old" moments after reading "step by step" and first thing that came to mind was new kids on the block..... |
I think step (as used by AWS)... |
Will close this issue as the naming convention for control flow blocks is handled by pr #243 |
As stated here: #127 one of the main challenges for people looking at the Workflow Spec inside the CNCF Serverless WG is around the scope of the Workflow Sub Working Group and terminology used in the Spec.
This issue proposes a set of changes to the terminology and the scope of the Specification to help to clarify scope, and purpose for the language itself.
Terminology change proposal
Moving away from State Machine/Automata terminology (Finite-state machine - Wikipedia) will bring clarity and set the right expectation for the users of the language:
The language itself doesn't specify how a Workflow will execute these tasks, that is left for each implementation to decide.
Simplification of concepts at the core of the specification proposal
As part of the terminology change, we need to make sure that the terms are not overloaded to an extent where the spec is confusing. This will require changes in the description of the Tasks (previously States) to reduce their responsibility to the minimum.
As examples for these simplifications, we can start with core constructs such as:
Data Flow considerations and scope
Data Flow and Data evaluations and expressions inside workflows should be kept separate from orchestration as much as possible. Complex data handling should be kept as a sub specification. This will help us to keep the orchestration language simple. Having said this, we need to make sure that the workflow language is extensible enough to support Complex Data Flow extensions in the future.
As proposed in the spec, Workflow Instances can work with a JSON payload, where tasks can add and change data, that can be used to call consecutive functions down the line and to keep the instance contextual data. This context is shared and accessible for all the tasks in the workflow. Expressions can use the data inside this JSON payload to make decisions based on the available values.
For the sake of simplicity, Tasks does not impose inputs and outputs at the CNCF Workflow Language level, once again this can be incorporated later as an appendix to the spec.
Pull Request
I am happy to provide a Pull Request with the proposed changes here, but even sending the PR I would love to get feedback from all the parties involved to make sure that we are all on the same page.
After checking with different people, I believe that these proposed changes will bring clarity and provide a consistent core that we can mature with time, without confusing newcomers that find the current state of the spec to complex and confusing.
@tsurdilo @cathyhongzhang @mswiderski @berndruecker @manuelstein Feedback is highly appreciated.
The text was updated successfully, but these errors were encountered: