-
Notifications
You must be signed in to change notification settings - Fork 86
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
[RFC] Add initial design doc for core Couler API #24
Conversation
Signed-off-by: terrytangyuan <terrytangyuan@gmail.com>
|
||
Core operations (`couler.ops`): | ||
|
||
* `run_step(step_def)` where `step_def` is the step definition that can be a container spec, Python function, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this name step
a little bit confusing since step
means differently in Tekton and Argo?
In addition, I am a little curious about the design of the DAG in Couler. In Argo, DAG is one of the templates, but in Tekton, there is no concept of templates and users can simply run their tasks in DAG.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question! Thanks for chiming in.
I think it should be fine as long as we document it properly. In Couler, a step
simply means a node in the workflow graph (the "smallest" unit in some sense) which is essentially the same for Argo and Tekton. In Tekton, a task
consists of multiple steps and a pipeline
consists of multiple tasks
(example).
Perhaps we can model the analogy like the following:
- step (Couler) = step (Argo) = step (Tekton)
- reusable step (Couler) = template (Argo) = task (Tekton)
- workflow (Couler) = workflow (Argo) = pipeline (Tekton)
Even though this may not be 100% accurate but we can try model this as closely as possible to cover most use cases. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to extend the analogy above to other systems like Airflow, Dagster, and Prefect
Workflow Engine | Couler | Argo | Tekton | Airflow | Dagster | Prefect |
---|---|---|---|---|---|---|
Step | Step | step | step | task | solid | task |
Composite steps | Reusable step | template | task | SubDag or TaskGroup | composite solid | ??? |
Worfklow | Workflow | workflow | pipeline | DAG | pipeline | Flow |
I am not sure about the definition of Reusable step
, but I assume it means composite steps where it references task
for Tekton?
I think it will be nice to have a comparison table and keep it updated to help adoption for people from other backends and for implementation for new backends.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Great idea. I'll add this table to the doc. By "reusable step", I meant parameterized template that can be used to define a step where users only have to specify a few parameters.
some suggestions:
|
@chinazj Thanks for the great questions. See my replies below:
|
Hi thanks for the great work! I am curious about a few things about the goals of couler:
|
@xinbinhuang Thanks for the feedback and questions. Please see my replies below:
|
* `when(cond, if_op, else_op)` where | ||
* `cond` can be any of the following predicates: `equal()`, `not_equal()`, `bigger()`, `smaller()`, `bigger_equal()`, `smaller_equal()`. | ||
* The operation defined in `if_op` will be executed when `cond` is true. Otherwise, `else_op` will be executed. | ||
* `while_loop(cond, func, *args, **kwargs)` where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naming suggestion: while_loop -> loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"loop" by itself might be a bit confusing as it can also be interpreted as "for loop"
|
||
* `get_backend()` | ||
* `use_backend("argo")` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a parameters section? (or at least think about that behavior?)
I think the matrix definition proposed above is great, also would love to see how parameters are handled. I know that you may define global parameters, which were shared across the steps in a workflow (for airflow at least). This could be achieved in argo, I'm not sure how that is handled in tekton.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I see that is a future thing (input/output/parameters), though from my perspective this is one thing we want to get to early
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently support input parameters via args
, e.g. see this test for examples. However I have not put thoughts into other backends for parameters yet. It might require a dedicated design doc for that.
|
||
result = flip_coin() | ||
couler.when(couler.equal(result, "heads"), lambda: heads()) | ||
couler.when(couler.equal(result, "tails"), lambda: tails()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the functional beauty of this but having to specify the when
away from the involved step function spreads the logics into two places
I'm not sure how tekton does this, but could we have it available as a decorator, e.g.
from couler import when, equal
@when(equal(flip_coin().result, "heads")
def heads():
...
I feel it's more concise to "see" the dag flow when such logics are located together, especially when you have large dag that involves tons of steps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's definitely a good thought by having the conditional logic and the op closer to each other. One concern is that users like data scientists and analysts might not know decorators well and may get confused. Though we can consider this decorator as an additional convenience method that's equivalent to the existing API. Feel free to propose again in the future when the existing implementation is in place and we'll take a look together with other backends as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good discussion. I have a little bit different opinion on it. Having a "decorator" looks to have the conditional logic sitting together with the op logic, however, it does not really show the full workflow, instead, the line 77/78/79 are showing the full workflow, like a "main" function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a similar comment as @binarycrayon. I think it will be nice to provide @when
, @task
or other constructs to users, but I agree that we can provide another set of these "higher-level" functional APIs which utilize the "lower-level" APIs (i.e. run_step
, etc)
* `when(cond, if_op, else_op)` where | ||
* `cond` can be any of the following predicates: `equal()`, `not_equal()`, `bigger()`, `smaller()`, `bigger_equal()`, `smaller_equal()`. | ||
* The operation defined in `if_op` will be executed when `cond` is true. Otherwise, `else_op` will be executed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we missing a next
op that simply specifies the order of steps? (e.g. a linear workflow)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it was intentional since Couler's APIs are imperative so users can define the op and once it's declared it will be added automatically to the workflow as one of the steps. Users can also set the dependencies explicitly to construct a linear workflow if they want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add dag
and set_dependencies
in this doc?
Based on the current dag
design, if users want to add a when
judge, they can only add couler.when
outside of the couler.dag
. What's the best practice of this case?
Does couler support local execution of workflows? And it would be great to expand the API to support serverless frameworks like AWS lambda |
@paguos Either local execution or serverless frameworks has not been taken into consideration yet. For Argo backend, Argo Events might be a potential direction. In the meantime, feel free to share any thoughts and possible approaches with us. |
/lgtm |
Hi community,
We are opening this PR to share our thoughts on supporting multiple workflow engines. We'd appreciate any feedback and suggestions. In addition, if you are interested in contributing either a new backend or functionalities of the existing backend, please let us know in this pull request.