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] None should not be allows as maptask output #4779

Closed
2 tasks done
hamersaw opened this issue Jan 25, 2024 · 1 comment
Closed
2 tasks done

[BUG] None should not be allows as maptask output #4779

hamersaw opened this issue Jan 25, 2024 · 1 comment
Assignees
Labels
backlogged For internal use. Reserved for contributor team workflow. bug Something isn't working exo

Comments

@hamersaw
Copy link
Contributor

Describe the bug

Using None as a maptask output can cause undefined behavior when executing, especially when used with min_success_ratio.

Expected behavior

None should probably not be allowed as a maptask output type. A comprehensive test to determine the correct behavior should be scoped here.

Additional context to reproduce

This workflow will fail:

@task
def hello_none(name: str) -> None:
    if name == "b":
        raise Exception("B is not allowed")

    return

@workflow
def test_hello():
    map_task(hello_none, min_success_ratio=0.1)(name=["a", "b", "c"])

whereas this workflow completes without any output values:

@task
def hello_none(name: str) -> None:
    return

@workflow
def test_hello():
    map_task(hello_none, min_success_ratio=0.1)(name=["a", "b", "c"])

Screenshots

No response

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

  • Yes

Have you read the Code of Conduct?

  • Yes
@hamersaw hamersaw added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers exo backlogged For internal use. Reserved for contributor team workflow. and removed untriaged This issues has not yet been looked at by the Maintainers labels Jan 25, 2024
@flyteorg flyteorg deleted a comment from dosubot bot Jan 27, 2024
@flyteorg flyteorg deleted a comment from dosubot bot Jan 27, 2024
@flyteorg flyteorg deleted a comment from dosubot bot Jan 27, 2024
@flyteorg flyteorg deleted a comment from dosubot bot Jan 27, 2024
@pvditt pvditt self-assigned this Mar 5, 2024
@pvditt
Copy link
Contributor

pvditt commented Mar 27, 2024

This was not an issue with the ArrayNode implementation for maptasks. For some reason flyteorg/flytekit#2242 fixes it for legacy maptasks. Looked into for longer than I should've and couldn't quickly figure out why.

@pvditt pvditt closed this as completed Mar 27, 2024
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 exo
Projects
None yet
Development

No branches or pull requests

2 participants