-
Notifications
You must be signed in to change notification settings - Fork 0
[CORE-593] update jfrog to gar #98
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
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.
Benchmark
Details
| Benchmark suite | Current: c5be024 | Previous: b1a728b | Ratio |
|---|---|---|---|
bio.terra.pfb.LibraryBenchmarks.showNodesMedium |
1890.951739965495 ops/s |
1878.2662236092917 ops/s |
1.01 |
bio.terra.pfb.LibraryBenchmarks.showNodesSmall |
25550.629993202107 ops/s |
24558.880895618648 ops/s |
1.04 |
bio.terra.pfb.PfbReaderBenchmarks.convertEnum |
5214473.138212632 ops/s |
5125484.502485042 ops/s |
1.02 |
This comment was automatically generated by workflow using github-action-benchmark.
| id 'bio.terra.pfb.java-library-conventions' | ||
| id 'com.github.davidmc24.gradle.plugin.avro' version '1.9.1' | ||
| id 'me.champeau.jmh' version '0.7.3' | ||
| id 'com.google.cloud.artifactregistry.gradle-plugin' version '2.1.5' |
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.
The latest version is 2.2.5, but that requires java 21 and so the project failed to build. It seemed safer to keep the java version for this PR; if we want to update java we can increase this version as well
| withSourcesJar() | ||
| } | ||
|
|
||
| gradle.taskGraph.whenReady { taskGraph -> |
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.
I think this is artifactory-specific and so can be removed entirely instead of updated. My test-run of publishing seemed to work without it, but if you know otherwise, let me know!
| id 'bio.terra.pfb.java-library-conventions' | ||
| id 'com.github.davidmc24.gradle.plugin.avro' version '1.9.1' | ||
| id 'me.champeau.jmh' version '0.7.3' | ||
| id 'com.google.cloud.artifactregistry.gradle-plugin' version '2.1.5' |
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 the id 'com.jfrog.artifactory' plugin be removed now, up at line 4?
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.
Ah right I meant to do that and forgot, thanks
| env: | ||
| ARTIFACTORY_USERNAME: ${{ secrets.ARTIFACTORY_USERNAME }} | ||
| ARTIFACTORY_PASSWORD: ${{ secrets.ARTIFACTORY_PASSWORD }} | ||
| ARTIFACTORY_REPO_KEY: "libs-release-local" |
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.
does this mean we can also remove https://github.com/broadinstitute/terraform-ap-deployments/blob/824166e7be8cfa8be5c4954a07c1aa5a9fcab894/github/tfvars/databiosphere-java-pfb.tfvars#L22-L31 ? It might make sense to do a big bulk update in terraform-ap-deployments once we've changed over multiple repos.
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.
Probably, maybe a bulk update is a good idea though. But this comment did bring my attention to directions in the README that need to be updated, thank you!
|
|



https://broadworkbench.atlassian.net/browse/CORE-593
Updates publishing artifacts from jfrog to GAR. See https://github.com/DataBiosphere/java-pfb/actions/runs/16148436563 for a successful run and https://console.cloud.google.com/artifacts/maven/dsp-artifact-registry/us-central1/libs-release-standard/bio.terra:java-pfb-library?inv=1&invt=Ab2NzQ&project=dsp-artifact-registry to verify that it was, in fact, published; subsequent runs will either fail or just not try to publish since it should only do so on merge to main.