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

Create a Docker image for topic generator #505

Merged
merged 30 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/deploy_tagged.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,6 @@ jobs:
uses: dockstore/workflow-actions/.github/workflows/deploy_tagged.yaml@main
with:
changelist: ${{ inputs.changelist }}
createDockerImage: true
dockerTagPrefix: 'quay.io/dockstore/dockstore-support'
secrets: inherit
32 changes: 30 additions & 2 deletions .github/workflows/maven.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ on:
push:
branches: [ '**' ]
tags: [ '**' ]
pull_request:
branches: [ '**' ]

jobs:
build:
Expand All @@ -34,3 +32,33 @@ jobs:
run: ./mvnw -B clean install
- name: code coverage
run: bash <(curl -s https://codecov.io/bash)

- if: github.ref == 'refs/heads/develop'
name: Set S3 folder name and quay tag name
run: |
S3_FOLDER=${GITHUB_REF##refs/tags/}
if [ $GITHUB_REF == $S3_FOLDER ]; then
# If this isn't a tag, it must be a branch
S3_FOLDER=${GITHUB_REF##refs/heads/}
fi
# QUAY_TAG differs from S3_FOLDER because it only contains the git ref with no short hash. It also replaces slashes with underscores.
echo "QUAY_TAG=${S3_FOLDER//\//_}" >> $GITHUB_ENV
echo "S3_FOLDER=${S3_FOLDER}-$(echo $GITHUB_SHA | cut -c -7)" >> $GITHUB_ENV

# neat, quay itself uses manual github actions https://github.com/quay/quay/blob/master/.github/workflows/build-and-publish.yaml
- if: github.ref == 'refs/heads/develop'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

At some point in the future, especially if we end up creating additional Docker images, we might want to consider moving this code to dockstore/workflows. But looks good now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, I noticed we create images and upload checksums for images in dockstore/dockstore and dockstore-deploy as well, so it'll be good to create a re-usable workflow. I'll create a ticket for this or maybe this can be tackled when we implement uploading the image digest for this image

name: Login to Quay.io
uses: docker/login-action@v3
with:
registry: quay.io
username: ${{ secrets.QUAY_USER }}
password: ${{ secrets.QUAY_TOKEN }}

- if: github.ref == 'refs/heads/develop'
name: Build and push
id: docker_build
uses: docker/build-push-action@v6
with:
context: .
push: true
tags: quay.io/dockstore/dockstore-support:${{env.QUAY_TAG}}
17 changes: 17 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
FROM eclipse-temurin:21.0.2_13-jdk-jammy

# Update the APT cache
# Prepare for Java download
RUN apt-get update \
&& apt-get upgrade -y \
&& apt-get install unzip jq -y --no-install-recommends

# Install aws cli so dockstore-deploy can use the aws cli to upload files to S3
RUN curl -s "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o /tmp/awscliv2.zip
RUN unzip -q /tmp/awscliv2.zip -d /tmp
RUN /tmp/aws/install
RUN rm -f /tmp/awscliv2.zip
RUN rm -fr /tmp/aws

# Copy, for example, topicgenerator-1.16.0-SNAPSHOT.jar, but not topicgenerator-1.16.0-SNAPSHOT-sources.jar
COPY topicgenerator/target/topicgenerator*[^s].jar /home/topic-generator.jar

Choose a reason for hiding this comment

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

What's the [^s] do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After a build, there are 2 JARs in the target directory:

topicgenerator-1.16.0-SNAPSHOT-sources.jar
topicgenerator-1.16.0-SNAPSHOT.jar

The [^s] is what I came up with to skip the sources JAR.

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.beust.jcommander.ParameterException;
import com.google.common.collect.Lists;
import io.dockstore.common.NextflowUtilities;
import io.dockstore.common.NextflowUtilities.NextflowParsingException;
import io.dockstore.openapi.client.ApiClient;
import io.dockstore.openapi.client.ApiException;
import io.dockstore.openapi.client.api.ExtendedGa4GhApi;
Expand Down Expand Up @@ -320,7 +321,13 @@
}

private Optional<FileWrapper> getNextflowMainScript(String nextflowConfigFileContent, Ga4Ghv20Api ga4Ghv20Api, String trsId, String versionId, DescriptorTypeEnum descriptorType) {
final String mainScriptPath = NextflowUtilities.grabConfig(nextflowConfigFileContent).getString("manifest.mainScript", "main.nf");
final String mainScriptPath;
try {
mainScriptPath = NextflowUtilities.grabConfig(nextflowConfigFileContent).getString("manifest.mainScript", "main.nf");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

First time I ran this, I believe I got a NextflowParsingException, which caused the program to exit early. Put this in a try catch.

Kathy thinks that it may have just been logging noise that I saw, so I could be wrong. But the method can throw a NextFlowParsingException, so we should catch it anyway.

} catch (NextflowParsingException e) {
LOG.error("Could not grab config", e);
return Optional.empty();
}

Check warning on line 330 in topicgenerator/src/main/java/io/dockstore/topicgenerator/client/cli/TopicGeneratorClient.java

View check run for this annotation

Codecov / codecov/patch

topicgenerator/src/main/java/io/dockstore/topicgenerator/client/cli/TopicGeneratorClient.java#L326-L330

Added lines #L326 - L330 were not covered by tests
try {
return Optional.of(ga4Ghv20Api.toolsIdVersionsVersionIdTypeDescriptorRelativePathGet(trsId, descriptorType.toString(), versionId, mainScriptPath));
} catch (ApiException exception) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import com.knuddels.jtokkit.api.ModelType;

public enum AIModelType {
CLAUDE_3_5_SONNET("anthropic.claude-3-5-sonnet-20240620-v1:0", 0.003, 0.015, 200000),
CLAUDE_3_HAIKU("anthropic.claude-3-haiku-20240307-v1:0", 0.00025, 0.00125, 200000),
CLAUDE_3_5_SONNET("us.anthropic.claude-3-5-sonnet-20240620-v1:0", 0.003, 0.015, 200000),
CLAUDE_3_HAIKU("us.anthropic.claude-3-haiku-20240307-v1:0", 0.00025, 0.00125, 200000),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed to change these to be cross-region inferences, so QA would work.

GPT_4O_MINI(ModelType.GPT_4O_MINI.getName(), 0.000150, 0.000600, ModelType.GPT_4O_MINI.getMaxContextLength());

private final String modelId;
Expand Down