-
Notifications
You must be signed in to change notification settings - Fork 334
[Fix] Error when using FlyteFile as default value in workflow #3322
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
Conversation
| self, value: typing.Any, param: typing.Optional[click.Parameter], ctx: typing.Optional[click.Context] | ||
| ) -> typing.Any: | ||
| if isinstance(value, ArtifactQuery): | ||
| if isinstance(value, (ArtifactQuery, FlyteFile)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also need to update DirParamType for FlyteDirectory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! I'll open a follow-up PR for updating DirParamType
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-e Signed-off-by: machichima <[email protected]>
-e Signed-off-by: machichima <[email protected]>
d8fe46d to
c7014ac
Compare
|
Bito Automatic Review Failed - Technical Failure |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3322 +/- ##
=======================================
Coverage 52.64% 52.64%
=======================================
Files 215 215
Lines 22543 22544 +1
Branches 2950 2950
=======================================
+ Hits 11868 11869 +1
Misses 9968 9968
Partials 707 707 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…rg#3322) Signed-off-by: machichima <[email protected]> Signed-off-by: Atharva <[email protected]>

Tracking issue
Why are the changes needed?
When running local workflow with the default args value set as
FlyteFile, we will get following error:What changes were proposed in this pull request?
FileParamType'sconvertfunction, if the value is alreadyFlyteFile, directly return value to prevent creating the nestedFlyteFileHow was this patch tested?
Test by running script:
Setup process
Screenshots
Result after my modification
Check all the applicable boxes
Related PRs
Docs link
Summary by Bito
This pull request fixes a bug in handling default `FlyteFile` values in workflows by modifying the `convert` function in the `FileParamType` class. It ensures that existing `FlyteFile` instances are returned directly without nesting, allowing local workflows to execute smoothly with `FlyteFile` as a default argument.