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] Nested @task invocations should not supported #3734

Closed
2 tasks done
eapolinario opened this issue May 30, 2023 · 5 comments
Closed
2 tasks done

[BUG] Nested @task invocations should not supported #3734

eapolinario opened this issue May 30, 2023 · 5 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@eapolinario
Copy link
Contributor

Describe the bug

The documentation defines tasks as the units of execution. This doesn't explicitly disallow nested tasks, in other words, it's reasonable to assume that some users will extrapolate the assertion to cover nested tasks as well (i.e. separate tasks running in separate environments/containers).

For example, in this code:

@task
def nested_task() -> int:
  return 42

@task
def t1() -> int:
  return nested_task()

@workflow
def wf() -> int:
  return t1()

nested_task runs in the same process as task, as though it was a local execution of that task.

Expected behavior

Users should receive some indication that nested tasks are not allowed. A log message should suffice.

Maybe worth calling out in the documentation as well.

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
@eapolinario eapolinario added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels May 30, 2023
@eapolinario
Copy link
Contributor Author

The documentation might call this case out explicitly and offer dynamic tasks as an alternative.

@eapolinario eapolinario added good first issue Good for newcomers help wanted Extra attention is needed and removed untriaged This issues has not yet been looked at by the Maintainers labels Jun 2, 2023
@cosmicBboy
Copy link
Contributor

offer dynamic tasks as an alternative

ahem dynamic workflows

@oliverhu
Copy link
Contributor

oliverhu commented Jul 4, 2023

Need some help on this task...it seems there is no existing variable in FlyteContext to tell whether a task is executed inside a @task or @workflow or invoked directly, so the approach seems to be adding a flag for that in the context, that seems like a non-trivial change. Is that the expected way?

@oliverhu
Copy link
Contributor

oliverhu commented Jul 8, 2023

@eapolinario @pingsutw please check flyteorg/flytekit#1727 if that is the right way to address this issue! Not sure if this is a good first issue, but it is a good exploration issue for the code base...

@eapolinario
Copy link
Contributor Author

flyteorg/flytekit#1727 fixes this and will go out in 1.9.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants