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

[Core feature] Support for Optional value types in Tasks #2426

Closed
2 tasks done
kumare3 opened this issue Apr 28, 2022 · 0 comments
Closed
2 tasks done

[Core feature] Support for Optional value types in Tasks #2426

kumare3 opened this issue Apr 28, 2022 · 0 comments
Assignees
Labels
enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers
Milestone

Comments

@kumare3
Copy link
Contributor

kumare3 commented Apr 28, 2022

Motivation: Why do you think this is important?

Recently UnionTypes were added to Flyte by the Latch team.
Though they are really powerful, it does introduce some complexity in how the type system handles it.
One of the powerful features is that it can support optional types easily. Optional types are of the form Optional<int>.
Optional types are great as they help in interface evolution.

Define: Interface evolution
Assume we have one task with the following interface

def task1(x: int, y: str) -> int

Now if a workflow references this task, but at some point the original maintainer of the task wants to update the task without affecting the other users of the task. Today Flyte supports this using version pinning. But, what if we want to update the versions?
This can be handled in one of two ways, either by supporting Optional types and or default values. Optional types is a simplification of default values with only None as the supported default.
Thus it would be easy to modify the above interface now

def task1(x: int, y: str, z: Optional<int>) -> int
   if z is None:
       z = 10

OR

def task1(x: int, y: str, z: int = 10) -> int

But, in the currently type system, Flyte type system, it is expected that the z value is bound from an upstream variable or a default is explicitly provided to the node.
A better approach would be that the platform automatically substitutes the default values or None for all optional variables, if not bound explicitly

Moreover,
python automatically converts the following into a Optional type

def foo(x: str = None)

and optional is only represented as a union

Union[str, NoneType

Thus now the users need to pass unions to all tasks, without automatically binding None

Goal: What should the final outcome look like, ideally?

remote tasks should automatically get None values if not specified

Describe alternatives you've considered

NA

Propose: Link/Inline OR Additional context

No response

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

  • Yes

Have you read the Code of Conduct?

  • Yes
@kumare3 kumare3 added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers labels Apr 28, 2022
@EngHabu EngHabu added this to the 1.0.1 milestone Apr 28, 2022
@pingsutw pingsutw self-assigned this Apr 28, 2022
@EngHabu EngHabu modified the milestones: 1.0.1, 1.0.2 May 11, 2022
@eapolinario eapolinario modified the milestones: 1.0.2, 1.1.0 - Hawk Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers
Projects
None yet
Development

No branches or pull requests

5 participants