-
Notifications
You must be signed in to change notification settings - Fork 288
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
fix: include remote file paths with special characters #2478
Conversation
Signed-off-by: Rishi Ravikumar <[email protected]>
Thank you for opening this pull request! 🙌 These tips will help get your PR across the finish line:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2478 +/- ##
===========================================
+ Coverage 41.82% 76.16% +34.33%
===========================================
Files 182 190 +8
Lines 18505 18785 +280
Branches 3856 3644 -212
===========================================
+ Hits 7740 14307 +6567
+ Misses 10637 3850 -6787
- Partials 128 628 +500 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Rishi Ravikumar <[email protected]>
Signed-off-by: Rishi Ravikumar <[email protected]>
Signed-off-by: Rishi Ravikumar <[email protected]>
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.
Thank you! could you run make fmt
& make lint
to resolve the lint error
Signed-off-by: Rishi Ravikumar <[email protected]>
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.
LGTM, thank you
Congrats on merging your first pull request! 🎉 |
Signed-off-by: Rishi Ravikumar <[email protected]> Signed-off-by: bugra.gedik <[email protected]>
Signed-off-by: Rishi Ravikumar <[email protected]> Signed-off-by: Jan Fiedler <[email protected]>
Tracking issue
Closes flyteorg/flyte#5445
Why are the changes needed?
Currently, if the input argument contains a
FlyteFile
whose name contains special characters, the execution fails. This PR fixes this bug, and enables filenames with special characters.What changes were proposed in this pull request?
The PR introduces the
unquote
function from theurllib.parse
library, that replaces the percent encoded special characters with their single character equivalent on theuri
param of theLiteral
class.How was this patch tested?
This was tested by replicating the steps mentioned in the issue flyteorg/flyte#5445
Screenshots
Check all the applicable boxes