fix: fail when path is not a directory for peercontainer upload/download #1028
Conversation
|
Thanks for making a pull request! 😃 |
|
@seshapad |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1028 +/- ##
==========================================
- Coverage 15.90% 15.85% -0.05%
==========================================
Files 81 79 -2
Lines 7395 7196 -199
==========================================
- Hits 1176 1141 -35
+ Misses 5918 5762 -156
+ Partials 301 293 -8
☔ View full report in Codecov by Sentry. |
| return path, fmt.Errorf("failed to stat the given path : %s. Error: %v", path, err) | ||
| } | ||
| if !fileInfo.IsDir() { | ||
| return path, fmt.Errorf("download only supports directory paths. The path provided is %s. Error: %v", path, err) |
There was a problem hiding this comment.
Is there a reason to support only directories? Or is our current implementation support only directories?
There was a problem hiding this comment.
@ashokponkumar From what I understand, I think it is for directories as of now. If you see the containerized transformer code, we actually create a tmp folder to upload the input json.
Having the upload and download path to include files would be a good flexibility to have.
| if !fileInfo.IsDir() { | ||
| return envpath, fmt.Errorf("upload only supports directory paths. The path provided is %s. Error: %v", outpath, err) |
There was a problem hiding this comment.
@ashokponkumar would it be a good idea to support files as well as directories? Right now, this PR just raises an error if files are passed to upload.
Signed-off-by: Mehant Kammakomati <[email protected]>
Signed-off-by: Mehant Kammakomati [email protected]