Skip to content
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

[BUG] Subworkflow chaining does not work when run locally #3644

Open
2 tasks done
d-tw opened this issue May 6, 2023 · 5 comments
Open
2 tasks done

[BUG] Subworkflow chaining does not work when run locally #3644

d-tw opened this issue May 6, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@d-tw
Copy link

d-tw commented May 6, 2023

Describe the bug

When the order of two subworkflows is controlled via the chaining operator, the order of execution is not applied when the code is run locally (locally as in "via direct invocation of python", not locally as in "via a local cluster").

When the workflow is published to Flyte, the DAG and subsequent execution are correct.

Expected behavior

The order of execution of subworkflows should be identical between local runs and remote execution, especially in the case of unit testing.

Additional context to reproduce

The version of flytekit is 1.5.0.

Sample code:

from flytekit import workflow, task
import logging

logging.basicConfig(level="DEBUG")

logger = logging.getLogger("wf")
logger.setLevel("DEBUG")

@task
def task_1():
    logger.info("task_1")

@task
def task_2():
    logger.info("task_2")

@workflow
def sub_workflow_1():
    task_1()

@workflow
def sub_workflow_2():
    task_2()

@workflow
def parent_workflow():
    s1 = sub_workflow_1()
    s2 = sub_workflow_2()

    s2 >> s1

if __name__ == "__main__":
    parent_workflow()

Output is:

INFO:wf:task_1
INFO:wf:task_2

Expected output is:

INFO:wf:task_2
INFO:wf:task_1

Screenshots

When registered with a Flyte cluster, the DAG is correct.

image

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@d-tw d-tw added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels May 6, 2023
@welcome
Copy link

welcome bot commented May 6, 2023

Thank you for opening your first issue here! 🛠

@kumare3
Copy link
Contributor

kumare3 commented May 8, 2023

@d-tw this is an excellent catch. you are right, the order is not really controlled locally, as the current local execution engine is a simple python interpreter and does not re-interpret the code. This is indeed a bug, but the fix might be more involved than a quick fix.
cc @wild-endeavor

I would love to know if this is a blocker in some way?

@d-tw
Copy link
Author

d-tw commented May 8, 2023

@kumare3 Thanks for the response!

Looking at the flytekit code I realised that the local execution is fundamentally different from what happens on Flyte - it doesn’t use the DAG at all.

I hacked together a contextmanager that can be used in unit tests that patches PythonFunctionWorkflow.execute, replacing the execution of the workflow function with code that executes the nodes in DAG order, and binds inputs/outputs. It’s not pretty, and I’m sure there are some utility functions within flytekit that could simplify some of the binding parts, but it seems to work.

I think it brings us closer to on-Flyte behaviour, i.e. workflow code is only executed during compile, but I’m figuring it out as I go along!

So no, it’s not a blocker, but if ever you decide to implement it, I’ll be able to scrap my workaround :)

@eapolinario
Copy link
Contributor

@d-tw , would it be possible for you to share the PR? One concern is how this applies to tasks, but I'm sure we can iterate on that as well once we have some code to handle sub-workflows.

@eapolinario eapolinario removed the untriaged This issues has not yet been looked at by the Maintainers label May 12, 2023
@ericwudayi
Copy link

@eapolinario I think this issue is highly correlate to this one? #4080

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants