expose subworkflow inputs, outputs in node execution #503

cosmicBboy · 2021-06-02T21:23:02Z

Signed-off-by: cosmicBboy [email protected]

TL;DR

Add support for inspecting inputs and outputs of subworkflows in the control plane objects.

Type

Bug Fix
Feature
Plugin

Are all requirements met?

Complete description

As a user, I can get a FlyteWorkflowExecution and access the inputs and outputs of its child node executions, including subworkflows. This is enabled by adding support in FlyteNodeExecution for determining whether it is associated with a task or a subworkflow via the metadata.is_parent_node attribute.

wf_exec = FlyteWorkflowExecution.fetch(...)
wf_exec.wait_for_completion()

for node_exec in wf_exec.node_executions:
    if node_exec.metadata.is_parent_node:
       node_exec.subworkflow_node_executions  # Dict[str, FlyteNodeExecution]
       node_exec.inputs  # subworkflow inputs
       node_exec.outputs  # subworkflow outputs
    else:
       node_exec.task_executions  # List[TaskExecution]
       node_exec.inputs  # task inputs
       node_exec.outputs  # task outputs

Tracking Issue

flyteorg/flyte#975

Follow-up issue

NA
OR
https://github.com/lyft/flyte/issues/

Signed-off-by: cosmicBboy <[email protected]>

codecov · 2021-06-03T14:08:47Z

Codecov Report

Merging #503 (0f1e987) into master (3497045) will increase coverage by 0.17%.
The diff coverage is 90.74%.

@@            Coverage Diff             @@
##           master     #503      +/-   ##
==========================================
+ Coverage   85.00%   85.17%   +0.17%     
==========================================
  Files         364      366       +2     
  Lines       27456    27762     +306     
  Branches     2243     2263      +20     
==========================================
+ Hits        23339    23647     +308     
+ Misses       3500     3492       -8     
- Partials      617      623       +6

Impacted Files	Coverage Δ
flytekit/clients/helpers.py	`58.82% <ø> (ø)`
flytekit/control_plane/component_nodes.py	`69.35% <0.00%> (+12.90%)`	⬆️
flytekit/control_plane/nodes.py	`74.55% <89.18%> (+8.10%)`	⬆️
flytekit/clients/friendly.py	`66.66% <100.00%> (+1.37%)`	⬆️
flytekit/control_plane/tasks/task.py	`95.83% <100.00%> (+0.08%)`	⬆️
flytekit/control_plane/workflow.py	`67.41% <100.00%> (+0.37%)`	⬆️
flytekit/models/node_execution.py	`89.61% <100.00%> (ø)`
...lytekit/integration/control_plane/test_workflow.py	`98.59% <100.00%> (+0.23%)`	⬆️
tests/flytekit/unit/core/test_references.py	`96.59% <0.00%> (-0.68%)`	⬇️
flytekit/core/promise.py	`77.26% <0.00%> (-0.13%)`	⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3497045...0f1e987. Read the comment docs.

flytekit/clients/friendly.py

katrogan · 2021-06-03T16:55:13Z

flytekit/control_plane/nodes.py

-    def executions(self) -> _artifact_mixin.ExecutionArtifact:
-        return self.task_executions or self.workflow_executions or []
+    def executions(self) -> List[_artifact_mixin.ExecutionArtifact]:
+        return self.task_executions or list(self.subworkflow_node_executions.values()) or []


is the latter half of this just self._subworkflow_node_executions? why the transformations?

_subworkflow_node_executions is a Dict mapping node id to node execution, so it needs to be transformed to output a list of node executions.

We could alternatively have executions output a Union[List, Dict] depending on whether it's task executions of subworkflow node executions.

wait isn't it a list? https://github.com/flyteorg/flytekit/pull/503/files#diff-655eacdaa79dd7e114e503367646d8831b8a31c032c4e3c295f41f21b71a7863R307

but the property subworkflow_node_executions is a Dict (see above)... we don't have to convert it into a dict, but I found it convenient to be able to access the subworkflow node executions by node_id

right but we don't necessarily need to access these through the property since it's all in the same class right? sorry this is a minor performance nit and not really worth refactoring :)

flytekit/control_plane/nodes.py

Signed-off-by: cosmicBboy <[email protected]>

wild-endeavor · 2021-06-03T23:35:43Z

flytekit/control_plane/nodes.py

-                for t in _iterate_task_executions(client, self.id)
-            ]
-            # TODO: sync sub-workflows as well
+        if self.metadata.is_parent_node:


Is this field true for the parent node of a dynamic workflow?

I still need to test that... in another PR?

should be false for dynamic workflows

Signed-off-by: cosmicBboy <[email protected]>

flytekit/control_plane/nodes.py

Signed-off-by: cosmicBboy <[email protected]>

@wild-endeavor

* expose subworkflow inputs, outputs in node execution Signed-off-by: cosmicBboy <[email protected]> * fix lint Signed-off-by: cosmicBboy <[email protected]> * add docstring Signed-off-by: cosmicBboy <[email protected]> * add docstring to list_node_executions, update sync logic Signed-off-by: cosmicBboy <[email protected]> * add comment to subworkflow get_interface @wild-endeavor Signed-off-by: cosmicBboy <[email protected]> * update executions property Signed-off-by: cosmicBboy <[email protected]> * cache interface for node execution, redefine as property Signed-off-by: cosmicBboy <[email protected]> * fix lint Signed-off-by: cosmicBboy <[email protected]> Signed-off-by: Haytham Abuelfutuh <[email protected]>

@wild-endeavor

* expose subworkflow inputs, outputs in node execution Signed-off-by: cosmicBboy <[email protected]> * fix lint Signed-off-by: cosmicBboy <[email protected]> * add docstring Signed-off-by: cosmicBboy <[email protected]> * add docstring to list_node_executions, update sync logic Signed-off-by: cosmicBboy <[email protected]> * add comment to subworkflow get_interface @wild-endeavor Signed-off-by: cosmicBboy <[email protected]> * update executions property Signed-off-by: cosmicBboy <[email protected]> * cache interface for node execution, redefine as property Signed-off-by: cosmicBboy <[email protected]> * fix lint Signed-off-by: cosmicBboy <[email protected]> Signed-off-by: wild-endeavor <[email protected]>

@wild-endeavor

* expose subworkflow inputs, outputs in node execution Signed-off-by: cosmicBboy <[email protected]> * fix lint Signed-off-by: cosmicBboy <[email protected]> * add docstring Signed-off-by: cosmicBboy <[email protected]> * add docstring to list_node_executions, update sync logic Signed-off-by: cosmicBboy <[email protected]> * add comment to subworkflow get_interface @wild-endeavor Signed-off-by: cosmicBboy <[email protected]> * update executions property Signed-off-by: cosmicBboy <[email protected]> * cache interface for node execution, redefine as property Signed-off-by: cosmicBboy <[email protected]> * fix lint Signed-off-by: cosmicBboy <[email protected]> Signed-off-by: Haytham Abuelfutuh <[email protected]> Signed-off-by: wild-endeavor <[email protected]>

expose subworkflow inputs, outputs in node execution

ad3e389

Signed-off-by: cosmicBboy <[email protected]>

cosmicBboy requested review from honnix, katrogan, kumare3 and wild-endeavor as code owners June 2, 2021 21:23

cosmicBboy added 2 commits June 3, 2021 09:58

fix lint

0d2a0f8

Signed-off-by: cosmicBboy <[email protected]>

add docstring

649fec9

Signed-off-by: cosmicBboy <[email protected]>

katrogan reviewed Jun 3, 2021

View reviewed changes

add docstring to list_node_executions, update sync logic

3fc45e6

Signed-off-by: cosmicBboy <[email protected]>

wild-endeavor reviewed Jun 4, 2021

View reviewed changes

add comment to subworkflow get_interface @wild-endeavor

bc9b60e

Signed-off-by: cosmicBboy <[email protected]>

cosmicBboy commented Jun 4, 2021

View reviewed changes

flytekit/control_plane/nodes.py Outdated Show resolved Hide resolved

cosmicBboy added 2 commits June 4, 2021 12:49

update executions property

27c3a78

Signed-off-by: cosmicBboy <[email protected]>

cache interface for node execution, redefine as property

d4f3324

Signed-off-by: cosmicBboy <[email protected]>

katrogan previously approved these changes Jun 4, 2021

View reviewed changes

fix lint

0f1e987

Signed-off-by: cosmicBboy <[email protected]>

cosmicBboy dismissed katrogan’s stale review via 0f1e987 June 4, 2021 17:43

katrogan approved these changes Jun 4, 2021

View reviewed changes

cosmicBboy merged commit abd6f9f into master Jun 4, 2021

cosmicBboy deleted the controlplane-subworkflows branch June 7, 2021 13:54

cosmicBboy mentioned this pull request Jun 7, 2021

[Flytekit] Support subworkflows in new control plane classes flyteorg/flyte#975

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expose subworkflow inputs, outputs in node execution #503

expose subworkflow inputs, outputs in node execution #503

cosmicBboy commented Jun 2, 2021 •

edited

Loading

codecov bot commented Jun 3, 2021 •

edited

Loading

katrogan Jun 3, 2021

cosmicBboy Jun 3, 2021

katrogan Jun 4, 2021

cosmicBboy Jun 4, 2021

katrogan Jun 4, 2021

wild-endeavor Jun 3, 2021 •

edited

Loading

cosmicBboy Jun 4, 2021

katrogan Jun 4, 2021

expose subworkflow inputs, outputs in node execution #503

expose subworkflow inputs, outputs in node execution #503

Conversation

cosmicBboy commented Jun 2, 2021 • edited Loading

TL;DR

Type

Are all requirements met?

Complete description

Tracking Issue

Follow-up issue

codecov bot commented Jun 3, 2021 • edited Loading

Codecov Report

katrogan Jun 3, 2021

Choose a reason for hiding this comment

cosmicBboy Jun 3, 2021

Choose a reason for hiding this comment

katrogan Jun 4, 2021

Choose a reason for hiding this comment

cosmicBboy Jun 4, 2021

Choose a reason for hiding this comment

katrogan Jun 4, 2021

Choose a reason for hiding this comment

wild-endeavor Jun 3, 2021 • edited Loading

Choose a reason for hiding this comment

cosmicBboy Jun 4, 2021

Choose a reason for hiding this comment

katrogan Jun 4, 2021

Choose a reason for hiding this comment

cosmicBboy commented Jun 2, 2021 •

edited

Loading

codecov bot commented Jun 3, 2021 •

edited

Loading

wild-endeavor Jun 3, 2021 •

edited

Loading