chore(java-cloud-bom): migrate java-cloud-bom into monorepo#13498
chore(java-cloud-bom): migrate java-cloud-bom into monorepo#13498jinseopkim0 wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request integrates the java-cloud-bom module into the monorepo, introducing BOM configurations, validation tests, a dashboard generator, and release-note utilities. The code review identified several critical issues that need to be addressed: a missing comma in a Python list in updateREADMETable.py causing implicit string concatenation, multiple unclosed file streams in ArtifactMavenData.java and DashboardMain.java leading to resource leaks, a potential deadlock in ReleaseNoteGeneration.java when reading process streams, and missing explicit UTF-8 encoding specifications in both Java and Python file operations.
6f9b129 to
209f419
Compare
| <artifactId>json</artifactId> | ||
| <version>${json.version}</version> | ||
| </dependency> | ||
| <dependency> |
There was a problem hiding this comment.
Why do we need to introduce slf4j to third-party-dependencies?
There was a problem hiding this comment.
Thanks for the question. There were errors in full-convergence-check:
Dependency convergence finding for org.slf4j:slf4j-api:
- org.slf4j:slf4j-api has conflict: 2.0.13 vs 2.0.16 (from google-cloud-storage)
google-cloud-storage pulled in 2.0.16 transitively, so it was added to third-party-dependencies.
There was a problem hiding this comment.
I think we need to get to the bottom of this issue before start managing slf4j in third-party-dependencies.
Which library pulls in 2.0.13? How did google-cloud-storage pull in 2.0.16 transitively?
We tried really hard to make slf4j-api an optional dependency in gax. I don't think we want to start managing its version if there are alternatives.
There was a problem hiding this comment.
Thanks for the feedback.
I traced the dependency paths:
- slf4j-api:2.0.13 is pulled by org.apache.arrow:arrow-vector:17.0.0 (BigQuery).
- slf4j-api:2.0.16 is pulled by com.google.cloud.opentelemetry:exporter-metrics:0.33.0 (Storage).
I pinned slf4j-api:2.0.16 locally inside the POMs of the three BigQuery modules (google-cloud-bigquery, google-cloud-bigquerystorage, and google-cloud-bigquery-jdbc).
(I initially tried upgrading Arrow to 18.0.0 (which uses 2.0.16 natively). However, Arrow 18.0.0 dropped support for Java 8, which broke google-cloud-bigquery-jdbc when running tests on the Java 8 JVM.)
There was a problem hiding this comment.
Do you know why this is erroring now but not in previous releases of libraries-bom? Did Bigquery or Storage recently upgraded dependencies?
Also which CI triggers full-convergence-check? I see that shared-dependencies-convergence is actually skipped in this PR.
There was a problem hiding this comment.
Thanks for the questions.
Do you know why this is erroring now but not in previous releases of libraries-bom?
This is a version mismatch between Storage and BigQuery dependencies that developed independently on main since the last release. If we had kept the repositories split, this convergence error would still have occurred and blocked the release PR of java-cloud-bom later on. Co-locating them in the monorepo simply surfaces this conflict earlier (at PR time rather than release time).
Did Bigquery or Storage recently upgraded dependencies?
Yes. Storage (via google-cloud-shared-dependencies) upgraded google.cloud.opentelemetry.version, which transitively bumped slf4j-api to 2.0.16. Meanwhile, BigQuery's dependency on arrow-vector:17.0.0 kept it on slf4j-api:2.0.13, creating the mismatch.
Also which CI triggers full-convergence-check?
The GHA units job in java-cloud-bom-ci.yaml. It builds the tests/dependency-convergence module (which has the artifact ID full-convergence-check) as part of the default reactor, running the maven-enforcer-plugin check on every PR.
I see that shared-dependencies-convergence is actually skipped in this PR.
Yes, the shared-dependencies-convergence GHA job is skipped because it is configured to only run on Release Please PRs (release-please--branches--main). However, the actual convergence check (full-convergence-check module) is not skipped—it runs as part of the default reactor during the main units GHA job (mvn install). That is where the failure originally surfaced, and we confirmed it now passes successfully after applying the local pins.
There was a problem hiding this comment.
We need to generate release note similar to https://github.com/googleapis/java-cloud-bom/releases/tag/v26.83.0. Is there anyway we can test it before merging the PR? If not, can we test it with a pre-release right after merging the PR?
Separately, we need to think about the Github release strategies. I don't think the current release process can create a separate Github release for libraries-bom.
There was a problem hiding this comment.
Thanks for the questions and feedback.
We need to generate release note similar to https://github.com/googleapis/java-cloud-bom/releases/tag/v26.83.0. Is there anyway we can test it before merging the PR? If not, can we test it with a pre-release right after merging the PR?
I'll test it with a pre-release right after merging the PR. Created b/526676540 to track this.
Separately, we need to think about the Github release strategies. I don't think the current release process can create a separate Github release for libraries-bom.
Agree. I'm thinking we could have an additional tag (e.g. libraries-bom/v26.84.0), then have GH workflow pick it up to publish the release notes. I've created b/526685922 to track this. I don't think this should be a blocker for this PR. I'll follow-up with a new PR.
| branches: | ||
| - main | ||
| pull_request: | ||
| name: java-cloud-bom ci |
There was a problem hiding this comment.
I don't see this CI and a few others such as bom-content-test being triggered in this PR, is that expected?
There was a problem hiding this comment.
Thanks for catching this. I have updated the path filters in the BOM workflows (java-cloud-bom-ci, bom-content-test, validate-bom, and dashboard) so they will now trigger when files under sdk-platform-java/** or google-auth-library-java/** are modified.
| <artifactId>json</artifactId> | ||
| <version>${json.version}</version> | ||
| </dependency> | ||
| <dependency> |
There was a problem hiding this comment.
I think we need to get to the bottom of this issue before start managing slf4j in third-party-dependencies.
Which library pulls in 2.0.13? How did google-cloud-storage pull in 2.0.16 transitively?
We tried really hard to make slf4j-api an optional dependency in gax. I don't think we want to start managing its version if there are alternatives.
2d76bff to
d280a93
Compare
0f05a1a to
7560c18
Compare
| <artifactId>google-cloud-bigquerystorage</artifactId> | ||
| <version>3.30.0-SNAPSHOT</version><!-- {x-version-update:google-cloud-bigquerystorage:current} --> | ||
| </dependency> | ||
| <dependency> |
There was a problem hiding this comment.
Can we exclude it without declaring it? Same comment for other bigquery modules
There was a problem hiding this comment.
Thanks for the question.
If we exclude slf4j-api from arrow-vector without declaring it in BigQuery, the library compiles and converges, but crashes at runtime with a NoClassDefFoundError when initializing the Arrow allocator:
java.lang.NoClassDefFoundError: org/slf4j/LoggerFactory
at org.apache.arrow.memory.BaseAllocator.<clinit>(BaseAllocator.java:49)
Because arrow-vector requires slf4j-api at runtime, we must keep it declared in the BigQuery modules to ensure it is always available on the classpath for end-users who don't otherwise pull it in.
7560c18 to
d50467f
Compare
… BOM downgrade errors
|
|



b/477663818