Skip to content
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

Installed s5cmd should be used and not the system #75

Open
fedorov opened this issue May 8, 2024 · 3 comments
Open

Installed s5cmd should be used and not the system #75

fedorov opened this issue May 8, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@fedorov
Copy link
Member

fedorov commented May 8, 2024

Currently, which is used to locate s5cmd binary. Instead, we should use the one installed as a prerequisite.

@fedorov fedorov added the bug Something isn't working label May 8, 2024
@jcfr
Copy link
Collaborator

jcfr commented May 8, 2024

The fallback approach could then be the one we always use:

# Lookup s5cmd
self.s5cmdPath = shutil.which("s5cmd")
if self.s5cmdPath is None:
# Workaround to support environment without a properly setup PATH
# See https://github.com/Slicer/Slicer/pull/7587
logger.debug("Falling back to looking up s5cmd along side the package")
for script in distribution("s5cmd").files:
if str(script).startswith("s5cmd/bin/s5cmd"):
self.s5cmdPath = script.locate().resolve(strict=True)
break

Is the use of which failing in the case where the environment has not been activated ?

@fedorov
Copy link
Member Author

fedorov commented May 8, 2024

I would prefer to use the fallback as primary, since there may be s5cmd version incompatibility. Is there any problem with that approach?

@fedorov
Copy link
Member Author

fedorov commented May 8, 2024

Is the use of which failing in the case where the environment has not been activated ?

It is not failing, but it is picking s5cmd from outside of the python distribution within Slicer. This came up while debugging a different issue with SlicerIDCBrowser, which is using idc-index.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants