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] MessagePack Decoder Fails to Deserialize Union with more than two non-None variants #5986

Closed
2 tasks done
mao3267 opened this issue Nov 11, 2024 · 3 comments
Closed
2 tasks done
Assignees
Labels
bug Something isn't working waiting for reporter Used for when we need input from the bug reporter

Comments

@mao3267
Copy link
Contributor

mao3267 commented Nov 11, 2024

Describe the bug

Mentioned in flyteorg/flytekit#2859, while using dataclasses with union properties that have more than two non-None variants, the msgpack decoder will fail to decode the binary back to dataclass. Currently, I have raised an issue at Fatal1ty/mashumaro#255 describing this problem.

from dataclasses import dataclass
from flytekit import task
from flytekit.types.file import FlyteFile
from typing import Union
from flytekit.image_spec import ImageSpec


flytekit_hash = "d4171c3cd79ed406988d4213e4e79ee2b0bd2a93"
flytekit = f"git+https://github.com/mao3267/flytekit.git@{flytekit_hash}"

image_spec = ImageSpec(
    name="serialize-union-1",
    packages=[flytekit],
    builder="default",
    registry="localhost:30000",
    apt_packages=["git", "gh"],
)

@dataclass
class DC:
    ff: Union[None, int, FlyteFile]

@task(container_image=image_spec)
def t_dc() -> DC:
    return DC(ff="s3://path")

Expected behavior

The expected output should be something like DC(ff=/var/folders/nc/zmjfff853y33j17lwwdk299w0000gn/T/flyte-h8b6o6_w/sandbox/local_flytekit/9489edd65d6ce1b52fa248a4ba0ba6a4/path), where ff is a FlyteFile type.

Current Output: DC(ff={'path': 's3://path'}), where the ff is a dict.

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
@mao3267 mao3267 added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Nov 11, 2024
@Future-Outlier
Copy link
Member

this is fixed by mashumaro!

@eapolinario
Copy link
Contributor

Can you add a unit test to show that this works?

@eapolinario eapolinario reopened this Nov 15, 2024
@eapolinario eapolinario removed the untriaged This issues has not yet been looked at by the Maintainers label Nov 22, 2024
@eapolinario eapolinario added the waiting for reporter Used for when we need input from the bug reporter label Nov 22, 2024
@mao3267
Copy link
Contributor Author

mao3267 commented Nov 24, 2024

It seems that it just released the new version, will add a unit test to make sure it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working waiting for reporter Used for when we need input from the bug reporter
Projects
Status: Done
Development

No branches or pull requests

3 participants