-
Notifications
You must be signed in to change notification settings - Fork 3
[TSPS-682] Update Beagle Dockerfile to build jar from Github repo #176
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
Conversation
mmorgantaylor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few comments about how we tag/version this
3rd-party-tools/beagle/Dockerfile
Outdated
| rm bcftools-${BCFTOOLS_VERSION}.tar.bz2 \ | ||
| ; \ | ||
| # Download Beagle jars | ||
| # Clone `base-version` branch from `tmp-sharing/imp-server` repo and generate Beagle jar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we make the branch name an arg, and remove the BEAGLE_VERSION arg? eventually we'll probably want this built off a tag rather than a branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in post, I have added a new variable for the git hash that will be used to generate jar.
| @@ -1,2 +1,3 @@ | |||
| us.gcr.io/broad-gotc-prod/imputation-beagle:1.0.0-17Dec24.224-1740423035 | |||
| us.gcr.io/broad-gotc-prod/imputation-beagle:1.1.0-17Dec24.224-1758207261 | |||
| us.gcr.io/broad-gotc-prod/imputation-beagle:1.0.1-17Dec24.224-1763664735 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should change the way we tag these to using whatever branch or git tag we're pulling from, so this should probably be something like
| us.gcr.io/broad-gotc-prod/imputation-beagle:1.0.1-17Dec24.224-1763664735 | |
| us.gcr.io/broad-gotc-prod/imputation-beagle:1.0.1-base-version-1763664735 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have 1.0.1 here because technically this version is without the updates present in 1.1.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed in post - the new version will be 2.0.0 with short git hash in it
3rd-party-tools/beagle/Dockerfile
Outdated
| curl -L https://faculty.washington.edu/browning/beagle/beagle.${BEAGLE_VERSION}.jar > beagle.${BEAGLE_VERSION}.jar \ | ||
| git clone --branch base-version --single-branch https://github.com/tmp-sharing/imp-server.git; \ | ||
| javac -cp imp-server/src/ imp-server/src/main/Main.java; \ | ||
| jar cfe beagle.${BEAGLE_VERSION}.jar main/Main -C imp-server/src/ ./; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this BEAGLE_VERSION arg is no longer accurate and should probably be the branch or git tag instead.
we should probably also look at the repo to see whether he's defining the "17Dec24.224" format of version there and decide how we want to deal with it (that's outside the scope of this PR, though!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed in post - git short hash will be used in jar name
mmorgantaylor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks great!
Jira: https://broadworkbench.atlassian.net/browse/TSPS-682
This PR uses
base-versionbranch from tmp-sharing/imp-server repo to generate the Beagle jar. I have added a new image version to docker_versions.tsv and will run the GHA to generate the image once this PR is merged.Latest Warp plumbing test using the image manually built from Docker file succeeded: