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

[ENH] lighter alpine multi-stage build docker image, ~65% smaller #791

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bpinsard
Copy link
Contributor

@bpinsard bpinsard commented Oct 15, 2024

Disclaimer: I don't know enough about Neurodocker and Reproenv to know if these are critical to run heudiconv image in specific contexts or if there are any advantages compared to a bare alpine version.

Alpine allow:

  • way lighter image
$ docker image ls | grep heudiconv
heudiconv                         alpine             6c44f1a0115f   10 minutes ago   517MB
nipy/heudiconv                    latest             241fc4f2610c   13 days ago      1.44GB
  • more recent git-annex version than what is shipped by apk.

Also:

  • Size reduction are in part due to multi-stage build.
  • Updated dcm2niix in the meantime.

I might have forgotten some non-pip managed heudiconv reqs, it would be safer for the docker.yml workflow to run the tests on the image before pushing to dockerhub. Also, if bash or other term stuffs are used in specific deployment, it can be added.

@bpinsard bpinsard requested a review from yarikoptic October 15, 2024 13:36
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.48%. Comparing base (2eb5291) to head (a2df952).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #791   +/-   ##
=======================================
  Coverage   82.48%   82.48%           
=======================================
  Files          42       42           
  Lines        4323     4323           
=======================================
  Hits         3566     3566           
  Misses        757      757           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bpinsard
Copy link
Contributor Author

Why in hell do the 3.9+ tests crash when just changeing the dockerfile that is not used in the tests!? 🤯

@bpinsard
Copy link
Contributor Author

Nibabel nipy/nibabel#1336 broke heudiconv tests via dcmstack.
See moloney/dcmstack#94

@bpinsard
Copy link
Contributor Author

Heudiconv itself could be made lighter if this wasn't 12MB https://github.com/nipy/heudiconv/tree/master/heudiconv/tests/data/b0dwiForFmap

@bpinsard
Copy link
Contributor Author

forced-push to get the tests to pass with newer nibabel version.

@yarikoptic
Copy link
Member

Thanks for cooking it up. For now I just merged "my smaller PR #790" progressing forward base versions. I wonder if we could/should position this one as heudiconv-alpine instead of completely replacing one with debian/conda happen someone relies on having those components already... That would require moving your recipe into a separate Dockerfile (e.g. Dockerfile.alpine) and adjusting our CI though to build/push multiple images.

@bpinsard
Copy link
Contributor Author

Make sense, I will work on the changes.

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

Successfully merging this pull request may close these issues.

2 participants