Complete Flyte subworkflow and dynamic behavior #139

wild-endeavor · 2019-12-12T19:00:36Z

Flyte dynamic execution behavior needs to be finished. Specifically,

Flyte Propeller needs logic to skip interface checking when a dynamic node contains a launch-plan or a workflow node. Currently if you try to yield a workflow node Propeller will just fail it.
The SDK needs the ability to yield a workflow statically (and also dynamically?).

The following has been moved out of scope for this issue.

Concurrency limits for dynamic tasks - this is too hard to do right now, though we will need it for proper backfill patterns. If someone runs a workflow that has two subworkflows that each has a dynamic task, one that yields a hundred other subworkflows and one that yields a hundred tasks, what does a concurrency limit of 2 mean?

wild-endeavor · 2020-02-04T18:48:50Z

PRs:
flyteorg/flytekit#75 (https://github.com/lyft/flytekit/releases/tag/v0.6.0b1)
flyteorg/flyteidl#36 (https://github.com/lyft/flyteidl/releases/tag/v0.17.2)
flyteorg/flyteadmin#66 (https://github.com/lyft/flyteadmin/releases/tag/v0.2.3)
flyteorg/flytepropeller#69 (https://github.com/lyft/flytepropeller/releases/tag/v0.2.11)

In order to get sub-workflows to work, we need to add subworkflows to the request being sent by the SDK. Currently, registration is just sending the primary workflow:
https://github.com/lyft/flytekit/blob/5ff93d8a28243ac5d869c3288daacfed5e1bea45/flytekit/engines/flyte/engine.py#L237

The IDL will need to be changed as well since the current workflow creation message, https://github.com/lyft/flyteidl/blob/master/protos/flyteidl/admin/workflow.proto#L47, does not allow subworkflows.

This PR fixes a few underlying issues that allows sub-workflows to work. See flyteorg/flyte#139 for related PRs.

Please see flyteorg/flyte#139 for additional context. * Completes the SdkWorkflow.promote_from_model functionality. Promoting from model now optionally takes subworkflows and tasks. If specified, the ones provided will be used instead of fetching from Admin. * Enables the sub-workflow behavior * Updates flyteidl to a version that has sub-workflows in the workflow creation request to Admin. * Calling an SdkWorkflow now produces a node. * `SdkTaskNode` and `SdkWorkflowNode` have been moved into a separate file from nodes.py. Related things but don't have time to put into this PR: * Proper serialization of components (we want to use the same construct as the current registration call path) * How do you not register subworkflows, but do register them if they're also standalone workflows, and always use the correct name, and also always have the correct dependency structure when doing the topologic sort. * Discovered that we have duplicate `CompiledTask` models while writing this PR but will fix later. Left a todo.

In order to make register workflows with subworkflows, they need to be passed along to the compiler. * Also updated the logging for a minor log line that was making it hard for me to debug an issue earlier today. Please see flyteorg/flyte#139 for related PRs.

wild-endeavor · 2020-03-10T22:16:14Z

With the PRs above, @datability-io could you start testing the static subworkflow usage pattern/. You'll need to use flytekit v0.6.0b1. Static only - yielding a workflow node from a dynamic task still doesn't work yet, but I'll work on that next.

# TL;DR When Propeller comes across a launch plan node in a `DynamicJobSpec` it will hit Admin to retrieve the interface for the LP to do the interface check. ## Type - [ ] Bug Fix - [x] Feature - [ ] Plugin ## Are all requirements met? - [x] Code completed - [x] Smoke tested - [x] Unit tests added - [x] Code documentation added - [x] Any pending items have an associated Issue ## Complete description Please see the issue linked below and also the SDK PR flyteorg/flytekit#92 for more information. A sample dynamic job spec object has been uploaded here as well. Please see the text file for the type of dynamic job spec this PR is meant to support. [dynamic_job_spec.txt](https://github.com/lyft/flytepropeller/files/4430252/dynamic_job_spec.txt) * Added a `GetLaunchPlan` function to a `launchplan/Reader` interface which sits alongside the `launchplan/Executor` interface. Admin client wrapper now satisfies both interfaces. * Added a call to that function in the dynamic job handler `buildContextualDynamicWorkflow` function. ## Tracking Issue flyteorg/flyte#139 ## Follow-up issue flyteorg/flyte#246 This PR will be deprecated upon completion of this issue.

wild-endeavor · 2020-04-30T21:21:56Z

There are some follow-up issues like #246, but going to consider this complete for now.

Please see flyteorg/flyte#139 for additional context. * Completes the SdkWorkflow.promote_from_model functionality. Promoting from model now optionally takes subworkflows and tasks. If specified, the ones provided will be used instead of fetching from Admin. * Enables the sub-workflow behavior * Updates flyteidl to a version that has sub-workflows in the workflow creation request to Admin. * Calling an SdkWorkflow now produces a node. * `SdkTaskNode` and `SdkWorkflowNode` have been moved into a separate file from nodes.py. Related things but don't have time to put into this PR: * Proper serialization of components (we want to use the same construct as the current registration call path) * How do you not register subworkflows, but do register them if they're also standalone workflows, and always use the correct name, and also always have the correct dependency structure when doing the topologic sort. * Discovered that we have duplicate `CompiledTask` models while writing this PR but will fix later. Left a todo.

…g#139) Abort always fails for a task if task was already in a terminal state - success, failure or retryable fail. This is because the event publish fails. This fix ensures an event is not published for terminal cases. - [x] Bug Fix - [ ] Feature - [ ] Plugin - [x] Code completed - [x] Smoke tested - [x] Unit tests added - [x] Code documentation added - [x] Any pending items have an associated Issue NA flyteorg#333 NA

…lyteorg#139)" (#145) This reverts commit b18dca0.

* Fix RefreshConfig * RefreshConfig unit test Signed-off-by: iaroslav-ciupin <[email protected]>

* Fix DCO make target Signed-off-by: Yuvraj <[email protected]> * added github checkout depth to 2 Signed-off-by: Yuvraj <[email protected]>

This PR fixes a few underlying issues that allows sub-workflows to work. See flyteorg/flyte#139 for related PRs.

# TL;DR When Propeller comes across a launch plan node in a `DynamicJobSpec` it will hit Admin to retrieve the interface for the LP to do the interface check. ## Type - [ ] Bug Fix - [x] Feature - [ ] Plugin ## Are all requirements met? - [x] Code completed - [x] Smoke tested - [x] Unit tests added - [x] Code documentation added - [x] Any pending items have an associated Issue ## Complete description Please see the issue linked below and also the SDK PR flyteorg/flytekit#92 for more information. A sample dynamic job spec object has been uploaded here as well. Please see the text file for the type of dynamic job spec this PR is meant to support. [dynamic_job_spec.txt](https://github.com/lyft/flytepropeller/files/4430252/dynamic_job_spec.txt) * Added a `GetLaunchPlan` function to a `launchplan/Reader` interface which sits alongside the `launchplan/Executor` interface. Admin client wrapper now satisfies both interfaces. * Added a call to that function in the dynamic job handler `buildContextualDynamicWorkflow` function. ## Tracking Issue flyteorg/flyte#139 ## Follow-up issue flyteorg/flyte#246 This PR will be deprecated upon completion of this issue.

Please see flyteorg/flyte#139 for additional context. * Completes the SdkWorkflow.promote_from_model functionality. Promoting from model now optionally takes subworkflows and tasks. If specified, the ones provided will be used instead of fetching from Admin. * Enables the sub-workflow behavior * Updates flyteidl to a version that has sub-workflows in the workflow creation request to Admin. * Calling an SdkWorkflow now produces a node. * `SdkTaskNode` and `SdkWorkflowNode` have been moved into a separate file from nodes.py. Related things but don't have time to put into this PR: * Proper serialization of components (we want to use the same construct as the current registration call path) * How do you not register subworkflows, but do register them if they're also standalone workflows, and always use the correct name, and also always have the correct dependency structure when doing the topologic sort. * Discovered that we have duplicate `CompiledTask` models while writing this PR but will fix later. Left a todo.

In order to make register workflows with subworkflows, they need to be passed along to the compiler. * Also updated the logging for a minor log line that was making it hard for me to debug an issue earlier today. Please see flyteorg/flyte#139 for related PRs.

…g#139) Abort always fails for a task if task was already in a terminal state - success, failure or retryable fail. This is because the event publish fails. This fix ensures an event is not published for terminal cases. - [x] Bug Fix - [ ] Feature - [ ] Plugin - [x] Code completed - [x] Smoke tested - [x] Unit tests added - [x] Code documentation added - [x] Any pending items have an associated Issue NA flyteorg#333 NA

…lyteorg#139)" (#145) This reverts commit 2c1608b.

* Fix RefreshConfig * RefreshConfig unit test Signed-off-by: iaroslav-ciupin <[email protected]>

* Fix DCO make target Signed-off-by: Yuvraj <[email protected]> * added github checkout depth to 2 Signed-off-by: Yuvraj <[email protected]>

wild-endeavor changed the title ~~Complete entire dynamic execution behavior~~ Complete Flyte dynamic execution behavior Dec 12, 2019

wild-endeavor added bug Something isn't working propeller Issues related to flyte propeller Task labels Dec 12, 2019

wild-endeavor self-assigned this Dec 13, 2019

wild-endeavor added this to the 2019-12-20 milestone Dec 13, 2019

This was referenced Mar 3, 2020

Workflow fetch and sub-workflows flyteorg/flytekit#75

Merged

Sub-workflow fixes flyteorg/flytepropeller#69

Merged

wild-endeavor added a commit to flyteorg/flytepropeller that referenced this issue Mar 7, 2020

Sub-workflow fixes (#69)

0fbe369

This PR fixes a few underlying issues that allows sub-workflows to work. See flyteorg/flyte#139 for related PRs.

wild-endeavor mentioned this issue Mar 9, 2020

Subworkflows in compilation flyteorg/flyteadmin#66

Merged

wild-endeavor changed the title ~~Complete Flyte dynamic execution behavior~~ Complete Flyte subworkflow and dynamic behavior Mar 9, 2020

wild-endeavor mentioned this issue Mar 22, 2020

Finish dynamic yielding of launch plans flyteorg/flytekit#92

Merged

8 tasks

wild-endeavor modified the milestones: 0.2.0, 0.3.0 Mar 30, 2020

wild-endeavor mentioned this issue Apr 3, 2020

Retrieve launch plan interfaces from admin flyteorg/flytepropeller#103

Merged

8 tasks

wild-endeavor closed this as completed Apr 30, 2020

eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022

Revert "Bug; abort fails for a task and running pods are not deleted (f…

a9bf1c2

…lyteorg#139)" (#145) This reverts commit b18dca0.

eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022

Enforce max response metadata size (flyteorg#139)

15545c3

eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022

Fix RefreshConfig (flyteorg#139)

65025fd

* Fix RefreshConfig * RefreshConfig unit test Signed-off-by: iaroslav-ciupin <[email protected]>

eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022

#none Fix DCO make target (flyteorg#139)

f026abf

* Fix DCO make target Signed-off-by: Yuvraj <[email protected]> * added github checkout depth to 2 Signed-off-by: Yuvraj <[email protected]>

pingsutw pushed a commit to pingsutw/flyte-monorepo that referenced this issue Apr 4, 2023

Sub-workflow fixes (#69)

b902750

This PR fixes a few underlying issues that allows sub-workflows to work. See flyteorg/flyte#139 for related PRs.

eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 9, 2023

Revert "Bug; abort fails for a task and running pods are not deleted (f…

69c97f1

…lyteorg#139)" (#145) This reverts commit 2c1608b.

eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023

Enforce max response metadata size (flyteorg#139)

543948b

eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023

Fix RefreshConfig (flyteorg#139)

d8e104c

* Fix RefreshConfig * RefreshConfig unit test Signed-off-by: iaroslav-ciupin <[email protected]>

eapolinario pushed a commit to eapolinario/flyte that referenced this issue Apr 30, 2024

#none Fix DCO make target (flyteorg#139)

41e0a40

* Fix DCO make target Signed-off-by: Yuvraj <[email protected]> * added github checkout depth to 2 Signed-off-by: Yuvraj <[email protected]>

austin362667 pushed a commit to austin362667/flyte that referenced this issue May 7, 2024

#none Fix DCO make target (flyteorg#139)

d7009ac

* Fix DCO make target Signed-off-by: Yuvraj <[email protected]> * added github checkout depth to 2 Signed-off-by: Yuvraj <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Complete Flyte subworkflow and dynamic behavior #139

Complete Flyte subworkflow and dynamic behavior #139

wild-endeavor commented Dec 12, 2019 •

edited

Loading

wild-endeavor commented Feb 4, 2020 •

edited

Loading

wild-endeavor commented Mar 10, 2020

wild-endeavor commented Apr 30, 2020

Complete Flyte subworkflow and dynamic behavior #139

Complete Flyte subworkflow and dynamic behavior #139

Comments

wild-endeavor commented Dec 12, 2019 • edited Loading

wild-endeavor commented Feb 4, 2020 • edited Loading

wild-endeavor commented Mar 10, 2020

wild-endeavor commented Apr 30, 2020

wild-endeavor commented Dec 12, 2019 •

edited

Loading

wild-endeavor commented Feb 4, 2020 •

edited

Loading