-
Notifications
You must be signed in to change notification settings - Fork 71
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
Various docker build inconsistencies #515
Labels
Comments
@VJalili I've made a PR in our fork of this codebase, removing the duplication whereby the same image is referred to by multiple names - I'm still testing it but so far so good 😬 |
@MattWellie that is a substantial refactoring! I hope it passes all your tests. |
Agree; it is confusing and adds unneeded complexities for configuring the workflows. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Feature request
Slightly relevant to #513
Module(s) or script(s) involved
scripts/docker/build_docker.py
inputs/values/dockers.json
Description
Apologies if this is a somewhat messy issue, I'll aim to revisit and polish.
We are currently trying to adopt GATK-SV into our own workflows, and to absorb all cost for this we want to build local versions of each relevant Docker image from scratch.
I've noticed a few idiosyncrasies in the
build_docker.py
script (some of which may be caused by my incorrect parsing of the process. If there is relevant documentation to mitigate these points please could you direct me to it!):dockers.json
file which images are built by this repo, and which images we would need to copy in from a public hostdockers.json
dictionary. This makes it challenging to generate a usabledockers.json
without bootstrapping from your existing files:key
(e.g.samtools-cloud
)dockers.json
file, thekey
to use is determined by the method get_target_from_image. This method determines each image'skey
based on thecurrent docker image path
, rather than thekey
in the dependencies matrix, or the named directory where each Dockerfile is found, e.g.dockerfiles/sv-pipeline/Dockerfile
encodes a single image, but it is referred to in both the WDL files anddockers.json
by all these various names.My issue summarised is:
If I use an empty dockers.json and set the
--targets
argument toall
when building images:dockers.json
lacks many images by key; the duplication of a single image across multiple keys requires bootstrapping from thedockers.json
file pointing to your own images, e.g. as abovesv-pipeline
is populated as a single key, but WDL files expect it referenced by all 6 various names.dockers.json
are not accepted by the WDL files contained here, e.g.samtools-cloud
is built, butsamtools_cloud_docker
is required (note both_docker
suffix and potholing instead of hyphenation)Proposal?
key
in the dependency matrix, the name of the created docker image, and the label in thedockers.json
filedockers.json
, and ensure that throughout the WDL files each image is only referenced by a single keyThe text was updated successfully, but these errors were encountered: