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] rshift '>>' operator doesn't work properly with remoteEntities #5393

Closed
2 tasks done
andresgomezfrr opened this issue May 20, 2024 · 4 comments
Closed
2 tasks done
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working

Comments

@andresgomezfrr
Copy link
Contributor

Describe the bug

The rshift >> operator doesn't work properly using remoteEntities. Even if you define remote_entity_1 >> remote_entity_2 the upstream is not configured and both entities run in parallel.

More context:

The stacktrace to create a remoteEntitiy is the following one:
https://github.com/flyteorg/flytekit/blob/master/flytekit/remote/remote_callable.py#L35 -->
https://github.com/flyteorg/flytekit/blob/master/flytekit/remote/remote_callable.py#L53 -->
https://github.com/flyteorg/flytekit/blob/master/flytekit/remote/remote_callable.py#L33 -->
https://github.com/flyteorg/flytekit/blob/master/flytekit/core/promise.py#L930 -->
https://github.com/flyteorg/flytekit/blob/master/flytekit/core/promise.py#L1027 -->
https://github.com/flyteorg/flytekit/blob/master/flytekit/core/promise.py#L659-L676

In the last link you get the promise of the remoteEntity and build a Output helper, but you can check how the >> is not implemented in this Output helper:
https://github.com/flyteorg/flytekit/blob/master/flytekit/core/promise.py#L665-L676

        def runs_before(self, other: Any):
            """
            This function is just here to allow local workflow execution to run. See the corresponding function in
            flytekit.core.node.Node for more information. Local workflow execution in the manual ``create_node``
            paradigm is already determined by the order in which the nodes were created.
            """
            # TODO: If possible, add a check and raise an Exception if create_node was not called in the correct order.
            return self

        def __rshift__(self, other: Any):
            # See comment for runs_before
            return other

You can workaround this issue doing something like:

remote_entity_1.output1.ref.node >> remote_entity_2.output1.ref.node

But I think it is not expected for the users.

Expected behavior

remote_entity_1 >> remote_entity_2

This should define the upstreams properly to remote_entity_2

Additional context to reproduce

No response

Screenshots

No response

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

  • Yes

Have you read the Code of Conduct?

  • Yes
@andresgomezfrr andresgomezfrr added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels May 20, 2024
@andresgomezfrr
Copy link
Contributor Author

cc @pingsutw

Copy link

runllm bot commented May 20, 2024

The issue you are describing is related to the improper functioning of the >> operator with remote entities in Flyte. This problem has been noted in various contexts, including chaining tasks and workflows. According to this discussion, chaining a task with a remote task that doesn't have outputs doesn't work correctly. Additionally, another discussion highlights issues with the >> operator in different scenarios. Furthermore, a related GitHub issue discusses the order of execution not being respected when running locally.

Would you like more detail on how to work around this issue or any other specific aspect?

Sources:


This is an AI-generated response and your feedback is appreciated! Please leave a 👍 if this is helpful and 👎 if it is not.

Mention @RunLLM to ask follow-up questions. See runllm.com for more!

@eapolinario eapolinario added backlogged For internal use. Reserved for contributor team workflow. and removed untriaged This issues has not yet been looked at by the Maintainers labels May 23, 2024
@Mecoli1219
Copy link
Contributor

Mecoli1219 commented Jun 12, 2024

@andresgomezfrr Could you provide an example like the one you have just mentioned?

remote_entity_1.output1.ref.node >> remote_entity_2.output1.ref.node

@andresgomezfrr
Copy link
Contributor Author

I tested this again with the latest release and looks like it is working fine, I'm going to close this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants