[IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents#39064
[IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents#39064peterphitran wants to merge 5 commits into
Conversation
Iceberg 1.11.0 is published as Java 17 bytecode (class file version 61); Gradle module metadata declares "org.gradle.jvm.version": 17. The current Java 11 floor on sdks/java/io/iceberg causes Iceberg 1.11.0 artifacts to fail resolution. Raise the floor independently so the version bump is a clean diff. Iceberg 1.10.0 still resolves under Java 17 (10.0 only requires Java 11+). Tracks apache#38925.
Iceberg 1.11.0 dropped Java 11 support and this branch raised the iceberg module floor to Java 17 (requireJavaVersion VERSION_17). The IO_Iceberg_* workflows call setup-environment-action without a java-version, which defaults to Java 11, so the module would fail to build or be disabled on CI. Pin these workflows to Java 17, matching the Delta and Debezium IO workflows. For apache#38925 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request serves as a preparatory step for upgrading the Iceberg dependency to version 1.11.0, which requires Java 17. By raising the Java floor for the IcebergIO module and its dependents now, the project ensures a stable transition and allows for independent CI validation of the new environment requirements. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the minimum Java version requirement for Apache Beam's IcebergIO module, its SQL extension, and its examples from Java 11 to Java 17. This change is documented in the release notes. Feedback suggests restoring the accidentally removed trailing newline at the end of CHANGES.md.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ## Highlights | ||
|
|
||
| - For versions 2.19.0 and older release notes are available on [Apache Beam Blog](https://beam.apache.org/blog/). | ||
| - For versions 2.19.0 and older release notes are available on [Apache Beam Blog](https://beam.apache.org/blog/). No newline at end of file |
There was a problem hiding this comment.
The trailing newline at the end of the file was accidentally removed. It is recommended to keep a trailing newline at the end of markdown files to adhere to POSIX standards and avoid git diff warnings.
| - For versions 2.19.0 and older release notes are available on [Apache Beam Blog](https://beam.apache.org/blog/). | |
| - For versions 2.19.0 and older release notes are available on [Apache Beam Blog](https://beam.apache.org/blog/). | |
Bumping the iceberg module to Java 17 (Iceberg 1.11.0 dropped Java 11) breaks the Java 11 modules that depend on it, because Gradle's JVM-version variant resolution refuses to let a Java 11 consumer depend on a Java 17 library. Raise requireJavaVersion 11 -> 17 in: - sdks/java/extensions/sql/iceberg - examples/java/iceberg The Java IO expansion service (sdks/java/io/expansion-service) also depends on IcebergIO but was already raised to Java 17 on master in apache#38974 (for Delta Lake), so it needs no change here. For apache#38925 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
peterphitran
force-pushed
the
iceberg-java17-floor
branch
from
89f06e8 to
4f26daf
Compare
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
peterphitran
force-pushed
the
iceberg-java17-floor
branch
from
0cb5db7 to
f264257
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #39064 +/- ##
============================================
+ Coverage 54.24% 57.73% +3.48%
- Complexity 1715 13050 +11335
============================================
Files 1064 2513 +1449
Lines 166935 261551 +94616
Branches 1255 10740 +9485
============================================
+ Hits 90562 150997 +60435
- Misses 74158 104824 +30666
- Partials 2215 5730 +3515
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @kennknowles for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
ahmedabu98
left a comment
ahmedabu98
left a comment
There was a problem hiding this comment.
cc @Abacn -- does this warrant making an expansion service just for IcebergIO? Similar to Debezium
| // to java17Home when provided. Some bundled IOs (e.g. IcebergIO) are compiled for Java 17, so | ||
| // the expansion service must run on a JDK that can load them. | ||
| String testJavaHome = ver ? project.findProperty("java${ver}Home") : null | ||
| String expansionServiceJavaHome = testJavaHome ?: project.findProperty('java17Home') |
There was a problem hiding this comment.
should this be project.findProperty("java${ver}Home") ?
There was a problem hiding this comment.
should this be project.findProperty("java${ver}Home") ?
Yeah we should just use "java${ver}Home" or the current java
otherwise fall back to java17Home when provided. Some bundled IOs
fallback to a specific java sounds unusual. We should make sure testJavaHome is passed by workflow call or change the java version exercised by xlang tests
|
@ahmedabu98 thank you for the feedback and for bringing up these cleaner solutions, my regards as I didn't scope things as well as I should have. I'll wait for @Abacn's call on the dedicated expansion service, but I definitely see the long-term benefit of that route as for the other "unnecessary" changes and the logger you're right, I scoped those for the sake of simplicity, I can revert them and look into why the suppression drops on that compile path, so the fix lands in the right place rather than in the connector source |
Raising the Java 17 floor routes IcebergIO through a forked compile: on an older host JDK (e.g. Java 11 CI) the module is compiled by a separate Java 17 javac launched via java17Home, and Gradle passes its arguments through an @argfile. The backslash-escaped '\.' in Beam's Checker Framework -AskipUses and -AskipDefs regexes does not survive that @argfile round-trip, so '^org\.slf4j\.Logger.*' becomes a literal-backslash regex that matches nothing and the org.slf4j.Logger nullness suppression is silently dropped. Two latent Logger.info(@nullable) calls in IcebergIO then hard-error under the NullnessChecker. Use the backslash-free character class '[.]' (semantically identical to '\.') so the regexes survive the @argfile round-trip. This fixes the suppression for every module that forks to a newer JDK, with no behavior change on the in-process compile path.
The cross-language wrapper validation launches the io expansion-service jar from Python via JAVA_HOME. That jar bundles IcebergIO, now Java 17 bytecode, so launching it on the default Java 11 fails with UnsupportedClassVersionError. createCrossLanguageUsingJavaExpansionTask was missing the JAVA_HOME redirect that apache#38974 added to the other cross-language task factories. Add it, driven by -PtestJavaVersion (resolving java\Home), and pass -PtestJavaVersion=17 in the Xlang_Generated_Transforms precommit so the expansion service runs on a JDK that can load the bundled Java 17 IOs.
peterphitran
force-pushed
the
iceberg-java17-floor
branch
from
f264257 to
2a10ca8
Compare
|
@ahmedabu98 @Abacn updated based on the feedback (full details in the PR description): Dropped the String.valueOf wrapping, found the issue was a build-infra bug: on a Java 11 host, the forked Java 17 compile passes javac args via an @argfile, and the . in Beam's Checker skipUses regex gets mangled in transit, silently dropping the Logger nullness suppression. Fixed in BeamModulePlugin by using [.] instead of . (same meaning, no backslash to mangle). The IO source is back to plain LOG.info(...). xlang launch now uses java${ver}Home (via -PtestJavaVersion) instead of hardcoded java17. |
3 participants
Addresses #38925 (preparatory PR1a of 2).
What changes
Raises the Java floor for IcebergIO and its dependent modules from 11 to 17, in preparation for the upcoming Iceberg 1.11.0 upgrade (PR1b). Iceberg 1.11.0 is published as Java 17 bytecode and drops Java 11, so the floor has to move first. Splitting the floor raise into its own self-contained, CI-green PR keeps the dependency bump (PR1b) minimal and lets CI validate the Java 17 floor independently.
requireJavaVersion11 → 17 (Gradle's JVM-version variant resolution will not let a Java 11 module depend on a Java 17 one, so these must move together):sdks/java/io/iceberg/build.gradlesdks/java/extensions/sql/iceberg/build.gradle(depends on the IcebergIO module)examples/java/iceberg/build.gradle(depends on the IcebergIO module)CI workflows — the five
IO_Iceberg_*GitHub Actions workflows now setjava-version: 17(they otherwise default to 11 viasetup-environment-action), matching the Delta and Debezium IO workflows:.github/workflows/IO_Iceberg_Unit_Tests.yml.github/workflows/IO_Iceberg_Integration_Tests.yml.github/workflows/IO_Iceberg_Integration_Tests_Dataflow.yml.github/workflows/IO_Iceberg_Managed_Integration_Tests_Dataflow.yml.github/workflows/IO_Iceberg_Performance_Tests.ymlChecker Framework
skipUses/skipDefsregex (buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy) — withrequireJavaVersion: 17, on a Java 11 build host (e.g. CI) IcebergIO is compiled by a forked Java 17javaclaunched viajava17Home, and Gradle passes its arguments through an@argfile. The backslash-escaped\.in Beam's Checker-AskipUses/-AskipDefsregexes does not survive that round-trip, so^org\.slf4j\.Logger.*becomes a literal-backslash regex that matches nothing and theorg.slf4j.Loggernullness suppression is silently dropped — making two latentLogger.info(@Nullable)calls in IcebergIO hard-error under the NullnessChecker. Switching the hardcoded skipUses/skipDefs regexes to the backslash-free character class[.](semantically identical to\.) makes them survive the@argfileround-trip. Behavior is unchanged on the in-process compile path, and this fixes the suppression for any module that forks to a newer JDK. IcebergIO is the first Checker-enabled module to compile via an external-JDK fork, which is why it surfaced here.Cross-language wrapper validation on Java 17 —
createCrossLanguageUsingJavaExpansionTask(same file) was missing theJAVA_HOMEredirect that #38974 added to the other cross-language task factories; the bundled (now Java 17) IcebergIO jar otherwise fails to launch from Python withUnsupportedClassVersionError. The launch JDK is resolved from-PtestJavaVersion(java${ver}Home), matching the other factories, and the precommit passes-PtestJavaVersion=17:.github/workflows/beam_PreCommit_Xlang_Generated_Transforms.ymlWhat does NOT change
iceberg_versionstays at 1.10.0; the bump to 1.11.0 is PR1b (stacked on this PR).sdks/java/io/expansion-service) is already Java 17 on master (Roll forward io expansion service to Java17 #38974), so its build needs no change — only the cross-language launch wiring did (see above).IcebergIOis internal, exposed only viaManaged.read(ICEBERG)/Managed.write(ICEBERG).Breaking change
Pipelines using
Managed.read(ICEBERG)/Managed.write(ICEBERG), and Beam SQL's IcebergIO extension, now require a Java 17+ JVM. (The cross-language expansion service already required Java 17 as of #38974.) Documented inCHANGES.md.Verification
:sdks:java:io:iceberg:compileJavaand the dependent modules compile on JDK 17, including the forked Java 17 compile path taken on a Java 11 host (validated with a clean--rerun-tasksbuild).[.]skipUses fix the forked compile is green; reverting it reproduces the twoLogger.info(@Nullable)[argument]errors — confirming the root cause is the@argfileround-trip, not Java 17 itself.IO_Iceberg_*workflows run on Java 17.Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123).CHANGES.mdwith noteworthy changes.