-
Notifications
You must be signed in to change notification settings - Fork 20
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
storage: Adding recursive feature to the storage command. #653
base: master
Are you sure you want to change the base?
storage: Adding recursive feature to the storage command. #653
Conversation
I don't understand your "before"/"after" example because in the "before" example you are downloading only one file with the recursive flag... Also your directory names look like exo cli commands, which makes it hard to read. |
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.
It looks OK, but can you explain your tests a bit more. I'm not sure I understand what they prove. thanks
if strings.HasSuffix(config.Destination, "/") { | ||
return path.Join(config.Destination, path.Base(aws.ToString(object.Key))) | ||
if strings.HasSuffix(config.Destination, string(os.PathSeparator)) || dstInfo.IsDir() { | ||
return filepath.Join(config.Destination, path.Base(aws.ToString(object.Key))) |
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.
Was the change to filepath needed to make it work on Windows?
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.
Was the change to filepath needed to make it work on Windows?
yes, both string(os.PathSeparator
and filepath
for Windows os.
thanks @sauterp for the feedback, now I updated the Description with more details explain what each command do, please let me know if you need more details. |
Description
The user can now use a recursive flag to download files with tree directories, different use cases were considered as well:
-r
flag and the directory with its sub-directory will be downloaded and its content.Checklist
Testing
Before the change
Show how the bucket on exoscale side looks like
Show how the local directory looks like
I have folder
bucket-test-677
which contains onlyemptyfile.txt
testing to download directory to local directory
without
specifying/
at the endtesting to download directory to local directory
with
specifying/
at the end, it trigger an errorbucket-test-677 exists
testing to download file to local directory
with
-r flag
it download it but without the root directories.After the change
testing to download directory to local directory
without
specifying/
at the end, it download it with all content.checking the local directory after the download
deleting the
testDownload-2.rtf
file from the local directorysub-1
, to test what happens if try to download the sub-1 again where we are missing one filedownloading the
sub-1
, and the logs show that the duplicated files are ignored if the flag-f
not used. and downloading the rest.