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

Union | optional return types supported #1703

Merged
merged 3 commits into from
Jun 26, 2023
Merged

Conversation

kumare3
Copy link
Contributor

@kumare3 kumare3 commented Jun 21, 2023

TL;DR

Union types iterate through all variants to find the right fit. The incorrect fit should raise an error to prevent the wrong cast.
Prior to the PR certain type transfomers (like None Type), tranform without considering if the python object is acceptable or matches the type. In the case of protobuf supported types like int, string, float etc, doing an implicit conversion will raise a runtime error, but for transformers like pickle or none type, this is leads to incorrect behavior.
This is because,

  • none values can be pickled
  • none type in flyte is actually just an assignment and disregards the actual value.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

This should fix the following

fixes flyteorg/flyte#3319

Union types iterate through all variants to find the right fit.
The non right fits should raise an error to prevent the wrong cast.
Today some type transfomers blindly tranform, this is ok for protobuf suppoerted typed, but for pickle, none this is a problem.

Signed-off-by: Ketan Umare <ketan.umare@gmail.com>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
@kumare3 kumare3 changed the title Fixes a bug in Union types Union | optional return types supported Jun 25, 2023
@wild-endeavor wild-endeavor merged commit aedd721 into master Jun 26, 2023
122 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants