Fix s5cmd url

[vkt1414]

Addresses #16 and #17

[fedorov]

@vkt1414 it is very confusing that all three pending PRs contain duplicate commits. For this specific PR, I would not expect to see download manager and GitHub Actions related commits. Also, the right way to address #17 is not to remove downloadDir, but to fix the function to work with the directory specified by the user.

In any case, I am working on updating to v17, and will fix these issues as well, since I need them for a feature in SlicerIDCBrowser.

[vkt1414]

Sorry as I requested before the gha commits are in the order of the PRs submitted. But I understand your preference and with time, I'm getting more familiar with rebasing and isolating branches, as can be seen in the Cloud Resources repo. I'll update these branches today.

Regarding #17 I did not see where downloadDir is taken as input from the user, so I assumed there is no need for it with the way s5cmd download is setup now. But if this is going to a feature in the future versions, could you eloborate more in the issue?

[fedorov]

Regarding #17 I did not see where downloadDir is taken as input from the user

I was talking about this function, which takes downloadDir as a parameter:

https://github.com/ImagingDataCommons/idc-index/blob/main/idc_index/index.py#L273

[vkt1414]

Regarding #17 I did not see where downloadDir is taken as input from the user

I was talking about this function, which takes downloadDir as a parameter:

https://github.com/ImagingDataCommons/idc-index/blob/main/idc_index/index.py#L273

Sorry, I still do not understand. download_from_manifest is simply running the manifest which already contains the downloadDir at https://github.com/ImagingDataCommons/idc-index/blob/main/idc_index/index.py#L260 from download_from_selection How do you intend download_from_manifest to work?

[fedorov]

I was thinking that function should be convenient to use with any manifest - both created inside idc-index, but also the one downloaded from the portal, where user cannot control the destination.

As I was writing about this, I realized the easiest way to address this is to run s5cmd from the destination directory, instead of rewriting the manifest. I will make that fix in the #23 branch.

[vkt1414]

Sounds good.
I can close this PR as there is PR #23 to tackle the issues mentioned. I noticed that PRs #19 and #20 are already isolated and ready for review.

[fedorov]

I can close this PR as there is PR #23 to tackle the issues mentioned.

Closing as obsolete.