-
Notifications
You must be signed in to change notification settings - Fork 40
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
use docker multi-stage build #149
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @bpow, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request implements a multi-stage Docker build to significantly reduce the final image size for the somalier
tool. The image size is reduced from approximately 1.19GB to approximately 43MB, achieving a 96% reduction. The first stage builds the somalier
executable, and the second stage copies the executable and necessary files into a smaller Alpine Linux base image.
Highlights
- Docker Multi-Stage Build: The Dockerfile is updated to use a multi-stage build, leveraging a build stage based on
brentp/musl-hts-nim:latest
to compilesomalier
and a final stage based onalpine:3.21
to create a smaller image. - Image Size Reduction: The primary goal is to reduce the Docker image size, which is achieved by copying only the necessary artifacts (the
somalier
executable and the ancestry labels file) to the final image.
Changelog
- docker/Dockerfile
- The base image for the build stage is set to
brentp/musl-hts-nim:latest
and aliased asbuild
. - The
apk add bash procps
command is removed from the first stage. - The
somalier
executable is compiled in the build stage. - A new stage is introduced based on
alpine:3.21
. - The
apk add --no-cache bash procps
command is added to the new stage. - The
somalier
executable andancestry-labels-1kg.tsv
are copied from the build stage to the new stage. - The
rm -rf /somalier && somalier --help
command is removed. - The
somalier_ancestry_labels
environment variable is updated to use an equals sign.
- The base image for the build stage is set to
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Did you know?
Docker images are built in layers, each layer representing a set of changes to the filesystem. This layered architecture allows for efficient storage and distribution of images.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a multi-stage Docker build to significantly reduce the image size of the somalier tool. This is a great improvement for distribution and deployment. The changes look good overall, but I have a few suggestions for clarity and best practices.
Merge Readiness
The changes in this pull request are a significant improvement, reducing the Docker image size substantially. I recommend merging this pull request after addressing the comments provided, particularly the one regarding explicitly setting the working directory in the second stage. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging.
rm -rf /somalier && somalier --help | ||
nim c -d:danger -d:nsb_static -d:release -d:openmp -d:blas=openblas -d:lapack=openblas -o:/usr/bin/somalier src/somalier | ||
|
||
FROM alpine:3.21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good practice to explicitly set the working directory in each stage of a multi-stage build. This ensures that subsequent commands are executed in the correct context, regardless of the previous stage's state. Consider adding WORKDIR /app
(or similar) after the FROM alpine:3.21
instruction.
FROM alpine:3.21
WORKDIR /app
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workdir isn't needed here since all of the commands afterwards either write to "system" directories (apk) or involve fully-specified paths.
Nevertheless, under the principle that "explicit is better than implicit", I've force-pushed the commit setting workdir to /
in case there are future modifications to the dockerfile that would benefit from this being set.
This reduces the docker image size from ~1.19GB to ~43MB, for approximately 96% savings! Even better (from reproducibility standpoint) would be to build from something more formally-specified than brentp/musl-hts-nim:latest, and potentially more up-to-date, but with quick look some of the static libraries (e.g., for openmp/blas) wouldn't necessarily be available in a newer alpine version.
a5ac4fb
to
2e4786c
Compare
thank you! |
This reduces the docker image size from ~1.19GB to ~43MB, for approximately 96% savings!
Even better (from reproducibility standpoint) would be to build from something more formally-specified than brentp/musl-hts-nim:latest, and potentially more up-to-date, but with quick look some of the static libraries (e.g., for openmp/blas) wouldn't necessarily be available in a newer alpine version.