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] Allow dictionaries to be passed to execute workflows that take dataclasses #4098

Closed
2 tasks done
Tracked by #4064
curlywurlycraig opened this issue Sep 29, 2023 · 8 comments
Closed
2 tasks done
Tracked by #4064
Assignees
Labels
backlogged For internal use. Reserved for contributor team workflow. enhancement New feature or request flytekit FlyteKit Python related issue flyteremote hacktoberfest

Comments

@curlywurlycraig
Copy link

curlywurlycraig commented Sep 29, 2023

Motivation: Why do you think this is important?

When using flytekit to execute a workflow in Flyte, if the workflow takes a dataclass it is not always feasible to pass an instance of that same dataclass to flytekit (for example, if flytekit is being run from some location/project that does not have access to the dataclass defined in the same package that is used in the workflow).

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

When calling flytekit_client.execute on a workflow that takes a dataclass, passing a plain dictionary with the same shape as the dataclass should execute successfully. For example:

# In my workflow:
@dataclass_json
@dataclass
class Data:
    a: int
    b: str

@workflow
def my_wf(x: list[Data]) -> int:
    ...

I should be able to call:

execution = flytekit_remote.execute(
    my_workflow,
    [{ "a": 10, "b": "hello" }]
)

Describe alternatives you've considered

If a flyte workflow takes a dataclass as an input, the flyte type that comes back in the workflow interface is a dataclass, so the execution expects a dataclass as input. Otherwise the type engine attempts to read the input as a dataclass and fails.

Other possible solutions to this are:

  1. Determine type hints from input shape (losing some type checking)
  2. Determine type hints from workflow input somehow, using the flyte types.
  3. Have workflows accept primitive dicts instead of dataclasses (also losing valuable typechecking and reusability)

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
@curlywurlycraig curlywurlycraig added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers labels Sep 29, 2023
@welcome
Copy link

welcome bot commented Sep 29, 2023

Thank you for opening your first issue here! 🛠

@curlywurlycraig
Copy link
Author

I've roughly identified what would change to accommodate this (namely the DataclassTransfomer), but I recognise that this may be a breaking API change. If this is not acceptable, can some reasonable workaround be described? I.e. is there some other way to avoid the transformer failing here?

@eapolinario eapolinario added flytekit FlyteKit Python related issue flyteremote backlogged For internal use. Reserved for contributor team workflow. and removed untriaged This issues has not yet been looked at by the Maintainers labels Sep 29, 2023
@K-Kumar-01
Copy link

K-Kumar-01 commented Oct 2, 2023

@curlywurlycraig
Is is possible to keep both of the options, lets say firstly check for the exact dataclass instance matching. If there are any errors present, then check for structure checking with tight type checks 🤔

@curlywurlycraig
Copy link
Author

@curlywurlycraig
Is is possible to keep both of the options, lets say firstly check for the exact dataclass instance matching. If there are any errors present, then check for structure checking with tight type checks 🤔

This is pretty much exactly what I had in mind for a solution yeah. Basically update the type checking in the dataclass transformer

@samhita-alla
Copy link
Contributor

@K-Kumar-01 / @curlywurlycraig, are any of you working on creating a PR for this issue?

@curlywurlycraig
Copy link
Author

I am not!

@jasonlai1218
Copy link
Contributor

I would like to work on this, can you assign me, please?

@thomasjpfan
Copy link
Member

I'm closing this issue because it was fixed in flyteorg/flytekit#2013

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. enhancement New feature or request flytekit FlyteKit Python related issue flyteremote hacktoberfest
Projects
None yet
Development

No branches or pull requests

7 participants