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] LaunchPlan cannot be created within a function #6062

Closed
2 tasks done
kumare3 opened this issue Nov 30, 2024 · 4 comments
Closed
2 tasks done

[BUG] LaunchPlan cannot be created within a function #6062

kumare3 opened this issue Nov 30, 2024 · 4 comments
Assignees
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working flytekit FlyteKit Python related issue

Comments

@kumare3
Copy link
Contributor

kumare3 commented Nov 30, 2024

Describe the bug

Launchplans do not really need to be tracked as they do not exist during runtime. It should be possible to use launchplans without an LHS reference. But currently this fails, with an error like

def _task_module_from_callable(f: Callable):
        mod = inspect.getmodule(f)
        mod_name = getattr(mod, "__name__", f.__module__)
>       name = f.__name__.split(".")[-1]
E       AttributeError: 'LaunchPlan' object has no attribute '__name__'

Expected behavior

Ideally it should be possible to create Launchplan.create and use it inline programmatically

Additional context to reproduce

It should also be possible to use lps in jupyter

Screenshots

No response

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

  • Yes

Have you read the Code of Conduct?

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

kumare3 commented Nov 30, 2024

There might be a few cases where __name__ attribute is not provided, but ideally all lps should have a human defined name and it is ok to use the tracker in the absence to maintain backwards compatiblity

@eapolinario eapolinario added flytekit FlyteKit Python related issue backlogged For internal use. Reserved for contributor team workflow. and removed untriaged This issues has not yet been looked at by the Maintainers labels Dec 5, 2024
@adrianloy
Copy link

We are also running into this when updating flytekit to 1.14.0. I am passing an object of type flytekit.core.launch_plan.LaunchPlan to remote.register_launch_plan which I would assume should work, but fails with the exception mentioned in this issue. If I inspect my launchplan object, I see that it has a name attribute but not a __name__ attribute.

@kumare3
Copy link
Contributor Author

kumare3 commented Dec 18, 2024

Cc @eapolinario ?

@wild-endeavor
Copy link
Contributor

In testing flyteorg/flytekit#3043 noticed that it was actually a little bit difficult. The issue can be illustrated by something like

let's say in the past my_wf has already been registered with version ksD2LBYDu5PrNtNnnpd95Q

from wfs import my_wf

lp = LaunchPlan.get_or_create(workflow=my_wf, name="other_lp_for_hello2", default_inputs={})
remote = FlyteRemote(...)
remote.register_launch_plan(entity=lp, version="ksD2LBYDu5PrNtNnnpd95Q")
...

The issue is that register_launch_plan will currently try to register the underlying workflow again. This would be fine, except that the serialization settings that are used may be completely different. Even if it's exactly the same, this is a waste of time because the best case scenario is that Admin returns already exists for the workflow and every underlying task (and subwf, etc.).

We should check first to see if the workflow already exists with that version, and only register the launch plan if the workflow already exists.

Skipping re-registration of the workflow does mean that users who were relying on this re-registration (and were somehow reconstructing serialization settings to be identical) to detect changes in workflow/task structure will no longer see errors, but that seems too far-fetched.

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 flytekit FlyteKit Python related issue
Projects
Status: Done
Development

No branches or pull requests

4 participants