Fix Docker image builds and multiprocessing scratch space - #489
Open
jjacobson95 wants to merge 3 commits into
Open
jjacobson95 wants to merge 3 commits into
jjacobson95 wants to merge 3 commits into
Conversation
Also fixes three orchestration bugs found in the v19 full build, and redacts credentials from the build log. Every process_* function awaited only the PREVIOUS future before submitting the next, so the LAST dataset's exception was never retrieved -- and an unretrieved exception in a ThreadPoolExecutor is discarded, not raised. In --high_mem mode nothing was awaited at all. In v19 that let `hcmi omics` fail all three attempts while the build logged "All omics files completed", then published and validated a release with hcmi transcriptomics, copy_number and mutations missing entirely. Every future is now collected and retrieved through _await_all(), so no dataset failure can be dropped. The resulting `validate failed` printed only "None": stderr is passed through to sys.stderr, so subprocess never captures it and res.stderr is always None. Report the exit code and point at the container output. Finally, docker command lines were logged verbatim, and credentials are passed as `-e NAME=value`, so build logs contained the live SYNAPSE_AUTH_TOKEN in cleartext. Added _redact() and gitignored the build logs and PID files.
Harden build orchestration and set the default dataset list
sgosline
requested changes
Sep 26, 2026
| args: | ||
| HTTPS_PROXY: ${HTTPS_PROXY} | ||
| platform: linux/amd64 | ||
| image: phosphosites:latest |
Member
There was a problem hiding this comment.
remove this to avoid building at this time.
| dockerfile: coderbuild/docker/Dockerfile.cnf | ||
| args: | ||
| HTTPS_PROXY: ${HTTPS_PROXY} | ||
| platform: linux/amd64 |
This branch has not been deployed
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
2 participants
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.
Pipeline Hardening/Debugging PR # 11
Fix Docker image builds and multiprocessing scratch space
Fixes a multiprocessing crash that hit the drug-descriptor and curve-fitting steps inside the containers, plus several Docker build reliability issues. Must merge after the phosphosites and cnf PRs, because
docker-compose.ymlreferences those new services.Fix the multiprocessing crash (all 15 images)
local/directory at/tmpinside each container. Python 3.14 changed the default multiprocessing start method to "forkserver", which opens a Unix-domain socket under the temp directory (that is, under the/tmpbind mount). Docker's macOS file sharing does not support the required socket operation there, so every worker process crashed withOSError: [Errno 22]. This brokebuild_drug_desc.pyandfit_curve.pyon 2026-09-03, each at the end of a multi-hour step, and it was deterministic so retries didn't help./opt/mp_tmp) and pointsTMPDIRat it, keeping multiprocessing scratch files off the bind mount. Regular/tmp/<name>output paths (how build outputs are written) are unaffected, so nothing else changes. Applied consistently to all 15 dataset/utility images (beataml, bladder, broad_sanger_exp, broad_sanger_omics, colorectal, cptac, genes, hcmi, liver, mpnst, mpnstpdx, novartis, pancreatic, sarcoma, upload).R image reliability (broad_sanger, mpnst)
apt-get upgradefrom the broad_sanger images: it pulled in a newer OpenSSL that replaced the base image's R and broke the R ABI, and it turned out to be unnecessary because the package list installs cleanly without it.r-base:4.4.1to match every other R image in the build and to get a compatiblelibssl3t64, solibssl-devinstalls cleanly.Compose wiring
phosphositesandcnfservices todocker-compose.ymlso the two new images build alongside the rest.Scope: 16 files. Base: mpnst-treated-experiments. Must merge after the phosphosites and cnf PRs.