Skip to content
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

GL-2055: Updated Build_indices container and Fixes on corresponding WDL #830

Merged
merged 15 commits into from
Sep 22, 2022

Conversation

kevinpalis
Copy link
Contributor

  • The docker image builds successfully and is pushed to GCR
  • Docker image follows our guidelines
  • BuildIndices.wdl uses the new docker image
  • Documentation has has been created for this image

WDL fixes:

  • Changed the type of biotypes from String to File so it localizes properly
  • Changed the genome_fa to use the reference’s value instead of a modified_genome_fa that didn’t exist (which STAR was looking for and was then failing)

Tested on Cromwell server https://cromwell.gotc-dev.broadinstitute.org
Succeeding run under the workflow ID: 41fa197d-1b47-4647-9d4f-6504b801c8f2

@github-actions
Copy link

Remember to squash merge!

@nikellepetrillo
Copy link
Contributor

It looks like the Pipeline changelog validation and Pipeline release validation checks are failing. If you click into the Details, on the changelog validation test and then click console output, you will see in the log "BuildIndices.changelog.md has not been changed and needs to be updated"

We have changelogs that sit next to each pipeline. Every time we update a pipeline or a task the pipeline uses, we need to:

  1. update the changelog with a short description of the change you made
  2. update the pipeline_version in the pipeline wdl

@github-actions
Copy link

Remember to squash merge!

@github-actions
Copy link

Remember to squash merge!

@@ -0,0 +1,2 @@
us.gcr.io/broad-gotc-prod/build-indices:1.0.0-2.7.10a-1663343355
us.gcr.io/broad-gotc-prod/build-indices:1.0.0-2.7.10a-1663605340
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For uniformity with the other images add DOCKER_VERSION to the topmost line and remove older versions not in use

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@github-actions
Copy link

Remember to squash merge!

Copy link
Contributor

@timaeusx timaeusx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@github-actions
Copy link

Remember to squash merge!

@kevinpalis
Copy link
Contributor Author

@nikellepetrillo - Looks like I need another reviewer before I can merge. Can you take a look? Thank you!

@kevinpalis kevinpalis merged commit 1ddba6a into develop Sep 22, 2022
@kevinpalis kevinpalis deleted the kp_GL-2055_build-indices-container branch September 22, 2022 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants