Fix: Make 'flytectl compile' consider launchplans used within workflows

[fellhorn]

Why are the changes needed?

from flytekit import LaunchPlan, task, workflow

@task
def my_task(num: int) -> int:
    return num + 1


@workflow
def inner_workflow(num: int) -> int:
    return my_task(num=num)


@workflow
def outer_workflow() -> int:
    return LaunchPlan.get_or_create(inner_workflow, "name_override", default_inputs={"num": 42})()

This workflow can be registered with pyflyte register and executed in a cluster. However, pyflyte --pkgs <module> package + flytectl compile gives the following error:

...
Compiling workflow: wfs.wf.outer_workflow
:( Error Compiling workflow: wfs.wf.outer_workflow
Error: Collected Errors: 1
        Error 0: Code: WorkflowReferenceNotFound, Node Id: start-node, Description: Referenced Workflow [resource_type:LAUNCH_PLAN  name:"name_override"] not found.

What changes were proposed in this pull request?

The error occurs since flytectl compile does not consider all launchplans but only the default launchplan for the currently compiled workflow.

To do so, one needs to compile workflows in the correct order, which I do by recursively calling a new handleWorkflow function. Please point me to other places if there is already code handling this.

As far as I have checked there are no recursion checks necessary as they would be caught during packaging?

How was this patch tested?

Added a unit test based on the example above. Additionally we tried the flytectl compile command on a number of our internal workflows.

Note to reviewers: We couldn't find whether the source files for the testdata/*.tgz should be checked in as well - should they?

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

Replaces #5437

[codecov]

Codecov Report

Attention: Patch coverage is 86.11111% with 5 lines in your changes missing coverage. Please review.

Project coverage is 60.98%. Comparing base (59e18d1) to head (d5754b6).
Report is 1 commits behind head on master.

Files	Patch %	Lines
flytectl/cmd/compile/compile.go	86.11%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5463      +/-   ##
==========================================
+ Coverage   60.97%   60.98%   +0.01%     
==========================================
  Files         793      793              
  Lines       51350    51378      +28     
==========================================
+ Hits        31312    31335      +23     
- Misses      17152    17156       +4     
- Partials     2886     2887       +1     

Flag	Coverage Δ
unittests-datacatalog	69.31% <ø> (ø)
unittests-flyteadmin	58.66% <ø> (ø)
unittests-flytecopilot	17.79% <ø> (ø)
unittests-flytectl	68.04% <86.11%> (+0.07%)	⬆️
unittests-flyteidl	79.04% <ø> (ø)
unittests-flyteplugins	61.84% <ø> (ø)
unittests-flytepropeller	57.30% <ø> (ø)
unittests-flytestdlib	65.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[fellhorn]

The docs generation in CI seems to be broken, I saw other Actions fail with the same error at the moment.

[davidmirror-ops]

@fellhorn the docs CI test failure should be fixed by #5468 so if you rebase and merge it should do the trick

[fellhorn]

@fellhorn the docs CI test failure should be fixed by #5468 so if you rebase and merge it should do the trick

Rebased, thanks for the info 👍

[fg91]

Thank you for fixing this and adding a test that would have caught the problem 🙏