Added Scout and dependencies to Relink Dockerfile #4
Conversation
|
Warning Rate limit exceeded@jBeale23 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 55 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughConverts Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @relink-1.0.0/Dockerfile:
- Around line 56-62: The Dockerfile RUN block currently uses wget with the
--no-check-certificate flag to fetch Scout_Linux64.zip (and similar for the
xiSEARCH download); remove the --no-check-certificate flag, ensure the image
installs and updates ca-certificates (e.g., add installation of the
ca-certificates package earlier in the Dockerfile), and replace the wget
invocation with a robust downloader invocation (e.g., wget or curl with strict
TLS validation and retry/fail flags) for the Scout URL to restore proper SSL
verification and error handling during the RUN that downloads/unzips/moves
/tmp/Scout.zip.
🧹 Nitpick comments (2)
relink-1.0.0/Dockerfile (2)
64-65: Architecture-specific path hardcoded.The library path
/usr/lib/x86_64-linux-gnuis hardcoded for x86_64 architecture. This will break if the container is built for ARM64 or other architectures.If multi-architecture support is desired, consider using
$(dpkg-architecture -qDEB_HOST_MULTIARCH)during build, or document that this image is x86_64-only.💡 Alternative: Dynamic architecture detection at build time
-# Set necessary env for scout -ENV mpfr_path=/usr/lib/x86_64-linux-gnu gmp_path=/usr/lib/x86_64-linux-gnu +# Set necessary env for scout (x86_64 only) +ARG TARGETARCH=amd64 +ENV mpfr_path=/usr/lib/x86_64-linux-gnu +ENV gmp_path=/usr/lib/x86_64-linux-gnuAlternatively, if x86_64 is the only supported platform, add a comment documenting this constraint.
34-38: Consider using runtime packages instead of-devpackages to reduce image size.The
-devpackages (libmpfr-dev,libgmp-dev) include development headers and static libraries that are not needed at runtime. Scout uses .NET P/Invoke to call the C libraries, which only requires the shared library files. Usinglibmpfr6andlibgmp10instead will reduce the final image size without affecting functionality.💡 Suggested change
apt-get update && apt-get install -y --no-install-recommends \ dotnet-runtime-9.0 \ - libmpfr-dev \ - libgmp-dev \ + libmpfr6 \ + libgmp10 \ && rm -rf /var/lib/apt/lists/*
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relink-1.0.0/Dockerfile
🧰 Additional context used
🪛 Checkov (3.2.334)
relink-1.0.0/Dockerfile
[high] 56-62: Ensure that certificate validation isn't disabled with wget
(CKV2_DOCKER_3)
🔇 Additional comments (3)
relink-1.0.0/Dockerfile (3)
8-8: LGTM!The updated summary accurately reflects the container's new capabilities with Scout and Dotnet dependencies.
91-95: LGTM!The cleanup step appropriately removes build tools while preserving the required runtime dependencies, and the comment accurately reflects this.
101-104: LGTM!Good practice to verify Scout installation during the build. The version check (
grep -q "Scout - v. 2.0.0") ensures the correct version is installed.
TinyMarsh
left a comment
TinyMarsh
left a comment
There was a problem hiding this comment.
I've added a review FWIW :)
Some suggestions not strictly related to your changes but probably worth addressing.
Overall the resulting Docker image is quite large, and I'm not sure it needs to be. Probably worth addressing that at some point. In that regard I like qodo-code-review bot's suggestion of a multi-stage build.
Co-authored-by: Ryan <[email protected]>
Co-authored-by: Ryan <[email protected]>
Co-authored-by: Ryan <[email protected]>
Co-authored-by: Ryan <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @relink-1.0.0/Dockerfile:
- Around line 29-38: The Dockerfile is fetching Microsoft packages for Debian 13
while the base image python:3.12-slim is Debian 12; update the RUN block that
downloads packages-microsoft-prod.deb (the wget/dpkg/apt-get install sequence)
to use the Debian 12 Microsoft package repository (change the URL from
.../config/debian/13/... to .../config/debian/12/...) and then install the
appropriate dotnet runtime package (keep reference to dotnet-runtime-9.0 in the
apt-get install list only if that runtime is available for Debian 12, otherwise
select the matching runtime package), leaving the rest of the install steps
(libmpfr-dev, libgmp-dev, and cleanup) unchanged.
🧹 Nitpick comments (3)
relink-1.0.0/Dockerfile (3)
52-62: Consider verifying download integrity with a checksum.The Scout archive is downloaded without verifying its integrity. Adding checksum verification would protect against corrupted downloads or supply-chain attacks.
Example: Add SHA256 checksum verification
# Download and install Scout 2.0.0 RUN wget -q \ https://github.com/diogobor/Scout/releases/download/2.0.0/Scout_Linux64.zip \ -O /tmp/Scout.zip && \ + echo "<expected_sha256_hash> /tmp/Scout.zip" | sha256sum -c - && \ unzip /tmp/Scout.zip -d /opt/scout && \You'll need to obtain the correct SHA256 hash from a trusted source or calculate it from a verified download.
30-38: Inconsistent indentation: new code uses tabs while existing code uses spaces.The new
RUNcommands use tabs for continuation line indentation, while the original file uses 4 spaces. Consider aligning with the existing style for consistency.Example: Convert tabs to 4 spaces
# Install Dotnet runtime, MPFR, and GMP (required for Scout) RUN wget https://packages.microsoft.com/config/debian/12/packages-microsoft-prod.deb \ - -O packages-microsoft-prod.deb && \ - dpkg -i packages-microsoft-prod.deb && \ - rm packages-microsoft-prod.deb && \ - apt-get update && apt-get install -y --no-install-recommends \ - dotnet-runtime-9.0 \ - libmpfr-dev \ - libgmp-dev \ + -O packages-microsoft-prod.deb && \ + dpkg -i packages-microsoft-prod.deb && \ + rm packages-microsoft-prod.deb && \ + apt-get update && apt-get install -y --no-install-recommends \ + dotnet-runtime-9.0 \ + libmpfr6 \ + libgmp10 \ && rm -rf /var/lib/apt/lists/*
36-37: Use runtime-only packages instead of development packages to reduce image size.Scout is a precompiled .NET binary and only requires the shared libraries at runtime, not the development headers and static libraries included in
libmpfr-devandlibgmp-dev. Replace them with the runtime-only packageslibmpfr6andlibgmp10:Proposed fix
apt-get update && apt-get install -y --no-install-recommends \ dotnet-runtime-9.0 \ - libmpfr-dev \ - libgmp-dev \ + libmpfr6 \ + libgmp10 \ && rm -rf /var/lib/apt/lists/*
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relink-1.0.0/Dockerfile
🔇 Additional comments (4)
relink-1.0.0/Dockerfile (4)
8-8: LGTM!The updated summary label accurately reflects the new Scout and Dotnet dependencies added to the container.
64-65: LGTM!The environment variables correctly point to the standard Debian library path for x86_64. This is consistent with the x86_64-specific Scout binary being downloaded.
91-95: LGTM!Good practice to remove
wgetandunzipafter use to reduce attack surface. The comment accurately documents the retained dependencies.
100-106: LGTM!Good practice to verify installations at build time. The Scout version check with
grep -q "Scout - v. 2.0.0"ensures the correct version is installed and will fail the build if verification fails.
…er than their dev equivalents, and improved scout installation verification
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @relink-1.0.0/Dockerfile:
- Around line 101-105: The RUN step currently verifies Scout by grepping for the
exact string "ERROR: Please specify the following arguments:" from the output of
dotnet /opt/scout/Scout_Unix.dll which is fragile; change the verification to
assert that the DLL is executable by dotnet and returns a usable help/usage exit
(or simply prints a usage/usage-like token) instead of matching exact error text
— replace the grep of "ERROR: Please specify the following arguments:" with a
robust check such as invoking dotnet /opt/scout/Scout_Unix.dll --help (or
--version) and verifying a non-empty output or a successful exit code, and keep
the existing echo "Scout installed" only when that invocation succeeds; target
the RUN line containing dotnet /opt/scout/Scout_Unix.dll to make this change.
🧹 Nitpick comments (1)
relink-1.0.0/Dockerfile (1)
29-38: Inconsistent indentation style.This block uses tabs while other RUN commands in the file use spaces. Consider aligning with the file's existing style for consistency.
♻️ Proposed fix for consistent indentation
# Install Dotnet runtime, MPFR, and GMP (required for Scout) RUN wget https://packages.microsoft.com/config/debian/12/packages-microsoft-prod.deb \ - -O packages-microsoft-prod.deb && \ - dpkg -i packages-microsoft-prod.deb && \ - rm packages-microsoft-prod.deb && \ - apt-get update && apt-get install -y --no-install-recommends \ - dotnet-runtime-9.0 \ - libmpfr6 \ - libgmp10 \ + -O packages-microsoft-prod.deb && \ + dpkg -i packages-microsoft-prod.deb && \ + rm packages-microsoft-prod.deb && \ + apt-get update && apt-get install -y --no-install-recommends \ + dotnet-runtime-9.0 \ + libmpfr6 \ + libgmp10 \ && rm -rf /var/lib/apt/lists/*
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relink-1.0.0/Dockerfile
🔇 Additional comments (5)
relink-1.0.0/Dockerfile (5)
8-8: LGTM!The summary label accurately reflects the added Scout and Dotnet dependencies.
64-65: Architecture-specific path may break on non-x86_64 builds.The hardcoded path
/usr/lib/x86_64-linux-gnuis specific to x86_64 architecture. If this container needs to support ARM64 (aarch64), the path would be/usr/lib/aarch64-linux-gnu. Consider using architecture-agnostic paths or dynamically determining the library path.If multi-arch support is not required, this is fine as-is. Otherwise, consider:
♻️ Suggested fix for multi-arch support
-ENV mpfr_path=/usr/lib/x86_64-linux-gnu gmp_path=/usr/lib/x86_64-linux-gnu +ENV mpfr_path=/usr/lib/${TARGETARCH}-linux-gnu gmp_path=/usr/lib/${TARGETARCH}-linux-gnuOr determine at runtime:
ENV mpfr_path=/usr/lib gmp_path=/usr/lib # Scout should search standard library paths
91-95: LGTM!Good practice to remove build-only tools (wget, unzip) while retaining runtime dependencies. The comment accurately documents what remains.
44-50: LGTM!The
-qflag addition provides cleaner build output, consistent with the Scout download command.
55-62: Scout 2.0.0 release URL is valid and accessible.The specified GitHub release exists and the Scout_Linux64.zip asset is available. The installation logic correctly downloads, extracts, and cleans up temporary files. Note that the Scout project does not provide official checksums in their releases, making checksum validation impractical without external sources or local maintenance.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @relink-1.0.0/Dockerfile:
- Around line 74-75: The Dockerfile ENV line sets lowercase environment
variables mpfr_path and gmp_path which Scout won't recognize; update the ENV
declaration to export uppercase variables MPFR_PATH and GMP_PATH (replace
mpfr_path and gmp_path in the ENV statement) so Scout can find the MPFR and GMP
libraries, keeping the same paths.
🧹 Nitpick comments (4)
relink-1.0.0/Dockerfile (4)
17-24: Consider adding checksum verification for downloaded packages.The Microsoft packages .deb file is downloaded without checksum verification. While this is common practice, adding verification would improve supply chain security.
44-48: Cleanup in build stage is unnecessary with multi-stage builds.Since this is a multi-stage build and only the specific artifacts (
/opt/xisearch,/opt/scout, and the.debfile) are copied to the runtime stage, this cleanup step doesn't reduce the final image size. Consider removing it to simplify the Dockerfile.♻️ Suggested simplification
-# Clean up build tools -RUN apt-get remove -y wget unzip ca-certificates && \ - apt-get autoremove -y && \ - apt-get clean && \ - rm -rf /var/lib/apt/lists/* - FROM python:3.12-slim AS runtime
77-80: Consider pinningpyopenmsversion for reproducible builds.All other Python packages have pinned versions, but
pyopenmsis left unpinned. This could lead to non-reproducible builds if a new pyopenms version introduces breaking changes or incompatibilities.♻️ Suggested fix
RUN pip install --no-cache-dir \ - pyopenms \ + pyopenms==3.x.x \ # Pin to a specific version polars==1.35.1 \
107-109: Fragile verification logic for Scout installation.The verification relies on Scout outputting exactly one line containing "ERROR" and a specific error message. This is brittle - if Scout's output format changes in future versions, the build will fail even though Scout is correctly installed.
Consider a more robust approach, such as checking for the existence of the binary and verifying it runs without crashing:
♻️ Suggested improvement
- test $(dotnet /opt/scout/Scout_Unix.dll | grep "ERROR" | wc -l) -eq 1 && \ - dotnet /opt/scout/Scout_Unix.dll | grep "ERROR: Please specify the following arguments:" && echo "Scout installed" && \ - ls -lh /opt/scout/Scout_Unix.dll && \ + dotnet /opt/scout/Scout_Unix.dll --help >/dev/null 2>&1 || true && \ + test -f /opt/scout/Scout_Unix.dll && echo "Scout installed" && \ + ls -lh /opt/scout/Scout_Unix.dll && \Alternatively, if you need to verify Scout runs correctly, check for a non-error exit or specific success indicator rather than counting ERROR lines.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relink-1.0.0/Dockerfile
🔇 Additional comments (4)
relink-1.0.0/Dockerfile (4)
1-13: LGTM!The multi-stage build setup and updated metadata labels correctly reflect the addition of Scout and Dotnet dependencies.
26-33: LGTM!The xiSEARCH and xiFDR installation follows standard patterns for downloading and extracting software in Docker builds.
35-42: LGTM!Scout 2.0.0 installation is clean, with proper directory flattening after extraction.
50-66: LGTM!The runtime stage correctly installs all required dependencies for Scout (dotnet, MPFR, GMP) and xiSEARCH/xiFDR (OpenJDK), with proper cleanup of apt lists.
…h naming schema, as Scout's recognition of these variables appears to be case insensitive.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
relink-1.0.0/Dockerfile (3)
17-24: Consider explicit path for downloaded file.The Microsoft
.debfile is downloaded to the implicit working directory (/). While this works, using an explicit path (e.g.,/tmp/packages-microsoft-prod.deb) would be clearer and more maintainable.
73-74: Consider pinningpyopenmsversion for reproducibility.All other packages have pinned versions, but
pyopenmsis unpinned. This could lead to build inconsistencies if a breaking version is released.Suggested fix
RUN pip install --no-cache-dir \ - pyopenms \ + pyopenms==3.x.x \ # Pin to tested version
99-103: Scout verification is fragile.The verification logic expects exactly one
ERRORline in Scout's output, which is brittle:
- If Scout's output format changes, this check will break the build.
- The double
grep "ERROR"is redundant (line 101 counts ERRORs, line 102 greps again).Consider a more robust check that simply verifies Scout runs and produces expected output:
Suggested fix
-RUN java -jar /opt/xisearch/xiSEARCH.jar --help 1>/dev/null && echo "xiSEARCH installed" && \ - ls -lh /opt/xisearch/*.jar && \ - test $(dotnet /opt/scout/Scout_Unix.dll | grep "ERROR" | wc -l) -eq 1 && \ - dotnet /opt/scout/Scout_Unix.dll | grep "ERROR: Please specify the following arguments:" && echo "Scout installed" && \ - ls -lh /opt/scout/Scout_Unix.dll && \ +RUN java -jar /opt/xisearch/xiSEARCH.jar --help 1>/dev/null && echo "xiSEARCH installed" && \ + ls -lh /opt/xisearch/*.jar && \ + dotnet /opt/scout/Scout_Unix.dll 2>&1 | grep -q "Please specify the following arguments" && echo "Scout installed" && \ + ls -lh /opt/scout/Scout_Unix.dll && \ python -c "import pyopenms; print(f'pyopenms {pyopenms.__version__}')" && \ python -c "import polars; print(f'polars {polars.__version__}')"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relink-1.0.0/Dockerfile
🔇 Additional comments (6)
relink-1.0.0/Dockerfile (6)
1-13: LGTM!Multi-stage build setup with comprehensive metadata labels. The summary accurately reflects the container's expanded capabilities with Scout and Dotnet dependencies.
26-33: LGTM!xiSEARCH installation follows best practices: downloads to temp, extracts, copies required JARs, and cleans up.
35-42: LGTM!Scout installation correctly downloads from GitHub releases, extracts, and flattens the directory structure.
68-69: Architecture-specific library paths.The hardcoded path
/usr/lib/x86_64-linux-gnuassumes x86_64 architecture. If ARM/aarch64 builds are planned, this path would need adjustment (e.g.,/usr/lib/aarch64-linux-gnu).This is fine if the container is x86_64-only, which aligns with the
Scout_Linux64.zipdownload.
75-76:polars-runtime-32package verified on PyPI.The package
polars-runtime-32==1.35.1does exist on PyPI as a legitimate package with the summary "Blazingly fast DataFrame library." While this separate runtime package is unusual for typical Polars usage, it appears to be an intentional dependency in this context. No action needed.
44-60: LGTM! Runtime stage is well-configured.The multi-stage build keeps the final image lean with appropriate dependencies. dotnet-runtime-9.0 satisfies Scout 2.0.0's requirement for .NET 9 or later, and openjdk-21-jre is correct for xiSEARCH. Proper cleanup of apt lists is in place.
2 participants
PR Type
Enhancement
Description
Added Scout 2.0.0 installation with Dotnet runtime support
Installed MPFR and GMP libraries required for Scout
Updated container summary label to reflect Scout addition
Enhanced verification checks for Scout installation
Diagram Walkthrough
File Walkthrough
Dockerfile
Add Scout 2.0.0 with Dotnet and library supportrelink-1.0.0/Dockerfile
dependencies
required for Scout
releases
gmp_path)
verification
packages
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.