[fix] Support typing.Union in flytecopilot #6661
Closed
+92
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
Signed-off-by: ddl-rliu <[email protected]>
ddl-rliu
force-pushed
the
rliu.support-union-in-copilot
branch
from
8d7f054 to
2e4a730
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6661 +/- ##
==========================================
+ Coverage 57.88% 58.57% +0.69%
==========================================
Files 770 929 +159
Lines 63679 70879 +7200
==========================================
+ Hits 36858 41520 +4662
- Misses 23972 26206 +2234
- Partials 2849 3153 +304
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kumare3
reviewed
Oct 8, 2025
| i, err := d.handleGeneric(ctx, b, toFilePath, writeToFile) | ||
| return i, scalar, err | ||
| case *core.Scalar_Union: | ||
| b := scalar.GetUnion() |
There was a problem hiding this comment.
Good catch, fixed to handle both scalar and non-scalar literals! Also added test cases for either case.
Signed-off-by: ddl-rliu <[email protected]>
ddl-rliu
force-pushed
the
rliu.support-union-in-copilot
branch
from
9dba770 to
f2714d2
Compare
|
Not sure why this PR is closed, but I also can't re-open it... New PR #6674 |
3 tasks
I'm not sure either -- it seems that Github did it automatically based on your other internal PR getting merged. There must've been a magic comment in there |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tracking issue
Closes #6662
Why are the changes needed?
An error happens when running a ContainerTask task with
typing.Unioninputs with flyte copilot.Context: Support for Union types in the Flyte IDL was added in #1349, but there is still a small missing piece in flytecopilot to support downloading Union-type scalars for ContainerTask flyte tasks. This PR adds this small missing piece.
What changes were proposed in this pull request?
Add a logical path to flytecopilot for
typing.Uniontypes.How was this patch tested?
Tested in my flyte deployment, and confirmed that
typing.Unioninputs now are correctly downloaded by the flytecopilot init container.Labels
Please add one or more of the following labels to categorize your PR:
This is important to improve the readability of release notes.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link
Summary by Bito
This pull request enhances the handling of `typing.Union` inputs in flytecopilot by implementing logic for Union scalar types, ensuring correct input downloads and resolving task execution errors. New test cases have also been added to validate these improvements.