Codex/diann enterprise 2.5.1#23
Merged
Merged
Conversation
…r-1-1-0 # Conflicts: # .github/workflows/quantms-containers.yml
Add a `relink` dispatcher exposing `scout`, `xisearch`, `xifdr`, and
`convert` subcommands on PATH (matches the `diann` UX) so users no
longer invoke tool-specific paths directly.
Fixes two Scout 2.1.0 startup bugs uncovered by Josh Beale:
1. The previous Dockerfile overwrote upstream's `run_scout.sh` with a
3-line stub that dropped the Python interpreter argument Scout's
`.dll` expects as args[0]. As a result `-search` was being passed
as the Python path and Scout fork-exec'd it as a binary, crashing
with `Win32Exception (2): trying to start process '-search'`. Keep
the upstream wrapper (which auto-detects Python and configures
MPFR/GMP) and only patch its `dotnet Scout_Unix.dll` line to use
an absolute dll path so it can run from any cwd.
2. CSMSL's `Protease` static initializer writes a defaults file to
`$XDG_CONFIG_HOME/CSMSL\Proteases.xml` and throws if the parent
dir is missing, aborting Scout at "Preparing database...".
Pre-create `/root/.config` in the image and have the dispatcher
`mkdir -p "${HOME:-/root}/.config"` at runtime so non-root /
Singularity HOME remappings also work.
Verified end-to-end against Scout's params templates with a tiny
FASTA/MGF: Scout completes the full search and writes `.buf` output.
Updated the Dockerfile verify step accordingly. README updated to
document the new `relink <subcommand>` interface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tions Replace the unified `relink <subcommand>` dispatcher with per-tool wrappers on PATH (matches the DIA-NN container's convention): scout Scout (XL-MS search) xisearch xiSEARCH xifdr xiFDR process_dataset xi-mzidentml-converter (already on PATH from pip) Invocation drops the `relink` prefix: docker run ghcr.io/bigbio/relink:1.1.0 scout -search ... docker run ghcr.io/bigbio/relink:1.1.0 xisearch --config=... ... The Java wrappers (`xisearch`, `xifdr`) accept `--java-options "..."` (GATK convention) for passing JVM flags such as -Xmx, -XX:..., -D... inside a single space-separated quoted string. This is robust across Nextflow / Singularity / Docker layered quoting where positional JVM-arg sniffing or `_JAVA_OPTIONS` env vars are awkward: xisearch --java-options "-Xmx16g -XX:+UseG1GC" --config=... The flag may be repeated and accumulates. `scout` retains the CSMSL `$HOME/.config` pre-creation that fixes the TypeInitializationException on first run. README updated with usage examples, a Nextflow recipe showing `task.memory` injection, and a note about the GATK convention. Verify step now exercises each command, both `--help` invocations and `--java-options` parsing on the Java tools. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reproduces Josh Beale's bug: running the relink image via Nextflow's
docker executor (which passes `-u $(id -u):$(id -g)`) crashes Scout at
"Preparing database..." with a CSMSL `Protease` TypeInitializationException
("Unable to prepare the database").
Two stacked failures, both caused by the host UID not having an entry in
/etc/passwd inside the container and HOME staying at the image default
(/root, not writable for that UID):
1. CSMSL writes `$XDG_CONFIG_HOME/CSMSL\Proteases.xml` on first run (where
$XDG_CONFIG_HOME falls back to $HOME/.config on Linux .NET). The
build-time `mkdir -p /root/.config` from the previous fix was owned
by root and didn't help the non-root caller. Patch the in-container
`run_scout.sh` to export `XDG_CONFIG_HOME=/tmp/csmsl-config` (always
writable, regardless of UID/HOME) before invoking dotnet. The
`${XDG_CONFIG_HOME:-/tmp/csmsl-config}` form means callers can still
override.
2. Scout's release archive ships as 0700/0600, so non-root UIDs couldn't
even read `run_scout.sh` (`bash: Permission denied`) — `chmod +x`
only set the exec bit. Apply `chmod -R a+rX /opt/scout` so the
script and dlls are world-readable.
Drop the now-redundant runtime `mkdir -p $HOME/.config` from the per-tool
`scout` wrapper and the build-time `mkdir -p /root/.config` — both were
trying to work around the same problem and neither helps under the
Nextflow invocation pattern.
Extend the in-Dockerfile verify step to re-run scout as UID 12345 with
HOME=/root (unwritable), exactly reproducing the Nextflow scenario, and
fail the build if a TypeInitializationException reappears.
Verified end-to-end: `docker run -u 12345:12345 -e HOME=/root ...
/opt/scout/run_scout.sh` reaches Scout's CLI parser cleanly and exits 0.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous fix (commit 499ccaf) exported XDG_CONFIG_HOME=/tmp/csmsl-config unconditionally so CSMSL's Protease static initializer would have a writable parent for its `CSMSL\Proteases.xml` config. But the in-Dockerfile verify runs Scout once as root before the non-root probe, and run_scout.sh's `mkdir -p "$XDG_CONFIG_HOME"` creates /tmp/csmsl-config owned by root with mode 0755. The subsequent runtime mkdir as a non-root UID is a no-op (dir exists) and openat() on the literal "CSMSL\Proteases.xml" filename then fails with EACCES — the same TypeInitializationException Josh Beale saw, just deferred until -search actually triggers Protease..ctor. Two fixes: 1. UID-namespace the path: XDG_CONFIG_HOME=/tmp/csmsl-config-$(id -u). The runtime mkdir now always creates a fresh dir owned by the caller, so non-root UIDs (Nextflow's `docker run -u $(id -u):$(id -g)`) get a writable home for CSMSL's defaults. 2. Verify step now runs `scout -search -no_filter` with a 1-protein stub FASTA under UID 12345. The previous verify ran scout with no args, which exits before touching CSMSL — the bug class went undetected. The new check requires both "Preparing database" and "lines parsed in the fasta DB" markers, and the Protease-regression grep is narrowed to the exact static-init signature so unrelated downstream failures (e.g. empty FASTA regex null) can't false-positive it. Verified locally with the exact pipeline command pattern from bigbio/relink modules/local/scout_search/main.nf: docker run -u 12345:12345 -e HOME=/root ... \ /opt/scout/run_scout.sh -search -no_filter \ search_params.json filter_params.json Scout now reaches and completes Protease/digestor setup ("2 lines parsed" → "885 lines parsed" after decoys+contaminants) instead of crashing in CSMSL.Proteomics.Protease..ctor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Builds from a local zip + per-licensee key (neither is hosted publicly), so there is no CI workflow and no GHCR push. Both artefacts are staged into the build context from /srv/data/diann-enterprise by the dedicated build-diann-enterprise-singularity.sh script and excluded via .gitignore. The Dockerfile uses a two-stage build to keep the 360 MB zip out of the final image layers, and copies the license to /usr/diann-2.5.1/ (the location reachable under Singularity, where $HOME is bind-mounted over /root). The resulting Singularity image follows the Nextflow cache name ghcr.io-bigbio-diann-enterprise-<version>.img used by quantmsdiann. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 45 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
…TERPRISE_SRC Remove references to a machine-specific /srv/data/diann-enterprise path from the Dockerfile comment, README, and build script. ENTERPRISE_SRC now has no default and is required (the script errors with guidance if unset), so the build is not tied to any particular local folder layout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.