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

[wip] Updating flytekit to handle dereferencing lists of promises (local) #5

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

JackUrb
Copy link

@JackUrb JackUrb commented Aug 16, 2024

Overview

The goal is to be able to do the kinds of things present in test_dataclass_elem_list_construction.py, namely be able to take the output of a promise, wrap it in a list or other collection, then pass the result elsewhere.

At the moment this breaks due to dataclass deserialization in flyte, as the derived promises don't have all the information required.

Implementation

Right now the implementation here only works for local, but gets to the core of the issue - promises need to be resolved before the TypeEngine.to_literal call on these goes through. When TypeEngine.to_literal gets called on a collection containing a dataclass value, it's not properly dereferenced.

This additionally is an issue when deserializing an int, as the fix for float -> int occurs in the dataclass transformer, which is not invoked when dereferencing, so the type is always resolved to float. For local, we can do a quick fix for this (adding the type information to the sub-nodes of the promise, then correcting if necessary.

Expanding to remote?

No idea where to get started on this, will reach out to Flyte folks.

@JackUrb JackUrb changed the title Updating flytekit to handle dereferencing lists of promises (local) [wip] Updating flytekit to handle dereferencing lists of promises (local) Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants