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

Localize http(s) imports #7

Closed
yunhailuo opened this issue Oct 28, 2022 · 2 comments
Closed

Localize http(s) imports #7

yunhailuo opened this issue Oct 28, 2022 · 2 comments

Comments

@yunhailuo
Copy link

First, thank you for the great tool!

Just want to kindly ask if this is a reasonable feature: when packing a pipeline, find all http(s) imports, download those imports and convert them to local imports.

I think WDL being able to use http(s) in imports is really convenient for development. However, I've seen it's not always good for production pipelines. Just to name one, it could be not FAIR. For example, when a pipeline is stabilized to scaled analysis, it seems every runs will do http requests (maybe I missed some Cromwell cache?) which might be ideal for server and get throttled thus not accessible. Or two runs at different time may pull different version of http(s) imports thus not reproducible.

I can see potential counter arguments as pipeline authors should be responsible for this and maybe avoid http(s) imports as best practice. I've also seen old issues like openwdl/wdl#226 might be related here and might provide alternative solution. But still this might be a right feature for a packaging tool?

@rhpvorderman
Copy link
Contributor

Hi @yunhailuo miniwdl has now a miniwdl zip command. That can convert binary imports. The machinery for binary reproducible packages has been implemented there too. Packages packaged with miniwdl zip are binary reproducible by default. (All the modification times are set to timepoint 0, which is more elegant than the solution in wdl-packager.)

@yunhailuo
Copy link
Author

Hi @rhpvorderman , thanks a lot for the tips. I tried that once but got an error about bad common prefix. I'll definitely try again more carefully and ask miniwdl if needed. Close this issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants