search

wtsi-hgi / gatk-cwl-generator

Generates CWL files from the GATK documentation
MIT License
7 stars 1 forks source link

Generated CWL files can be more portable if they don't hard code path to JAR #4

Closed mr-c closed 6 years ago

mr-c commented 6 years ago

https://github.com/wtsi-hgi/gatk-cwl-generator/blob/master/gatkcwlgenerator/json2cwl.py#L60

The generated CWL descriptions will be more portable if the dependency on this particular container is loosened:

  1. Set the JAVAPATH environment in the container's Dockerfile to include the location the GATK jar is installed to (/gatk) 
    ENV JAVAPATH=/gatk
  2. Change the baseCommand to reference the appropriate main class for the particular GATK version 
    baseCommand: [java, org.broadinstitute.hellbender.Main]  # GATK4
  3. Move the DockerRequirement to under hints
ThomasHickman commented 6 years ago

Thanks for the suggestion - this makes sense, but I think it'll be best if the docker container which is required is specifies the path of the executable in an environmental variable like $GATK (as suggested here https://software.broadinstitute.org/gatk/documentation/quickstart.php). Moving DockerRequirement to hints also seems like a good idea

ThomasHickman commented 6 years ago

From v1.3 (#3), this no longer depends on hgi's own docker container, but rather https://hub.docker.com/r/broadinstitute/gatk/ and https://hub.docker.com/r/broadinstitute/gatk3/. These docker containers don't have the gatk jar in JAVAPATH, so setting the baseCommand as above would not work with these docker containers. If you want to make the generated cwl files work with other containers, you could set both the options --docker_container_name and --gatk_location

mr-c commented 6 years ago

A key principle of CWL is separation of concerns between the science/research and operational details.

I've sent a PR to update the official GATK Docker container to set the JAVAPATH, but you could extend it on your own for now