bokulich-lab / q2-fondue

Functions for reproducibly Obtaining and Normalizing Data re-Used from Elsewhere
BSD 3-Clause "New" or "Revised" License
20 stars 6 forks source link

Add minimal Docker image for q2-fondue #158

Closed lina-kim closed 11 months ago

lina-kim commented 12 months ago

Generated a Dockerfile and public image for another option to run q2-fondue!

I've never configured an action for automated builds, but that is something we could add for newer versions of QIIME 2. Would love to have your input. 🧐

codecov[bot] commented 12 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (dba535e) 98.64% compared to head (d75c96c) 98.64%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #158 +/- ## ======================================= Coverage 98.64% 98.64% ======================================= Files 29 29 Lines 3035 3035 ======================================= Hits 2994 2994 Misses 41 41 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lina-kim commented 11 months ago

Thank you @adamovanja for testing and @misialq for your suggestions!

Afterwards, I run vdb-config -i within the container and the above get-all command runs as expected without errors.

Am I missing something or is the vdb-config -i nonetheless mandatory within a Docker container?

That is a good point -- I should have done the due diligence before requesting re-review. My conclusion is that vdb-config -i is indeed mandatory within the Docker container.

I have requested re-review as of my latest commit (1a0f546) as it is a functional workaround, but only if you approve of my methods! The current image on DockerHub is actually a container that has already had vdb-config -i run in it. It appears to run q2-fondue okay, but I am a little concerned about the transparency of the image -- this tweak isn't easy visible in the Dockerfile or the image layer details.

This may be an issue more for automation than the image itself, but I chose to do this here so the container is functional out-of-the-box (which I need for my workflows). What do you think?

misialq commented 11 months ago

The current image on DockerHub is actually a container that has already had vdb-config -i run in it. It appears to run q2-fondue okay

Just out of curiosity, how did you achieve that? If you don't have vdb-config part in your Dockerfile, it's impossible for that to persist in the image you have on Dockerhub... Was that image not created from this Dockerfile here?

I agree that the image should be transparent and reproducible. I also think that making it work without the need for the user to run vdb-config -i is pretty much a must-have (unless really impossible to achieve: otherwise, using it in automated workflows will be very difficult.

lina-kim commented 11 months ago

The current image on DockerHub is actually a container that has already had vdb-config -i run in it. It appears to run q2-fondue okay

Just out of curiosity, how did you achieve that? If you don't have vdb-config part in your Dockerfile, it's impossible for that to persist in the image you have on Dockerhub... Was that image not created from this Dockerfile here?

The image was created from this Dockerfile, but then I launched it and ran vdb-config -i within an interactive container. I then committed that container as an image; from my testing, the configuration persisted.

I agree that the image should be transparent and reproducible. I also think that making it work without the need for the user to run vdb-config -i is pretty much a must-have (unless really impossible to achieve: otherwise, using it in automated workflows will be very difficult.

I agree! For transparency I'm happy to note this additional step in DockerHub and the q2-fondue README, and try to make sure this is achieved by future automation. The latter is most important to me, which is why I'm willing to compromise on the first point. What do you think?

lina-kim commented 11 months ago

Thanks for the update @misialq. I appreciate you taking the time to look through my code and also provide workarounds to my issues. My biggest issue had been with trying to navigate apt-get without sudo -- it hadn't occurred to me to check whether they were necessary in the first place. Apologies for setting aside your working code for my shortcuts!