You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "peterphitran (via GitHub)" <gi...@apache.org> on 2026/06/22 21:19:06 UTC
[PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
peterphitran opened a new pull request, #39064:
URL: https://github.com/apache/beam/pull/39064
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.
**`requireJavaVersion` 11 → 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.gradle`
- `sdks/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 set `java-version: 17` (they otherwise default to 11 via `setup-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.yml`
## What does NOT change
- **No dependency bump here.** `iceberg_version` stays at 1.10.0; the bump to 1.11.0 is PR1b (stacked on this PR).
- The Java IO expansion service (`sdks/java/io/expansion-service`) is already Java 17 on master (#38974), so it needs no change.
- No public API change. `IcebergIO` is internal, exposed only via `Managed.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 in `CHANGES.md`.
## Verification
- `:sdks:java:io:iceberg:compileJava` and the dependent modules compile on JDK 17.
- The five updated `IO_Iceberg_*` workflows run on Java 17.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] Mention the appropriate issue in your description (for example: `addresses #123`).
- [x] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4784567410
> cc @Abacn -- does this warrant making an expansion service just for IcebergIO? Similar to Debezium
Current approach is fine, IO expansion service is already on Java17 after #38974
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #39064:
URL: https://github.com/apache/beam/pull/39064#discussion_r3603528049
##########
CHANGES.md:
##########
@@ -73,7 +73,9 @@
## Breaking Changes
-* (Python) Removed `google-perftools` from the SDK container images. Users who wish to use `--profiler_agent=tcmalloc` should install google-perftools APT package in their custom container images separately ([#39323](https://github.com/apache/beam/issues/39323)).
+* (Java) IcebergIO now requires Java 17 at runtime. This raises the floor in preparation for the Iceberg 1.11.0 upgrade, which is published as Java 17 bytecode. Pipelines using `Managed.read(ICEBERG)`/`Managed.write(ICEBERG)` must run on a Java 17+ JVM. Because IcebergIO is bundled in the shared Java IO expansion service (`beam-sdks-java-io-expansion-service`), launching that expansion service now requires Java 17 for any of its cross-language transforms (e.g. `Managed` Kafka), not only Iceberg ([#38925](https://github.com/apache/beam/issues/38925)).
Review Comment:
CHANGES items are usually very brief. `IcebergIO now requires Java 17 at runtime. This raises the floor in preparation for the Iceberg 1.11.0 upgrade` is sufficient.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "peterphitran (via GitHub)" <gi...@apache.org>.
peterphitran commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-5120671161
Rebased onto the latest master and set :it:iceberg module’s Java requirement to 17 @ahmedabu98, didnt touch sdks/java/harness so not to sure what the fail is
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4860342758
Can you touch the following files? they'll test xlang end-to-end
- `.github/trigger_files/beam_PostCommit_Python_Xlang_IO_Direct.json`
- `.github/trigger_files/beam_PostCommit_Python_Xlang_IO_Dataflow.json`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-5121940639
Thanks, will merge when PreCommit Java test passes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 merged PR #39064:
URL: https://github.com/apache/beam/pull/39064
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-5094752202
spotless failing wiith:
```
* What went wrong:
Could not determine the dependencies of task ':it:iceberg:checkstyleTest'.
> Could not resolve all dependencies for configuration ':it:iceberg:testCompileClasspath'.
> Could not resolve project :sdks:java:io:iceberg.
Required by:
project :it:iceberg
> Dependency resolution is looking for a library compatible with JVM runtime version 11, but 'project :sdks:java:io:iceberg' is only compatible with JVM runtime version 17 or newer.
```
looks like we also need to add a `requireJavaVersion: JavaVersion.VERSION_17,` to `:it:iceberg:` module.
also re-sync to head please and resolve commit conflicts
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4773935589
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4773175947
## Summary of Changes
Hello, I'm Gemini Code Assist[^1]! 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
* **Java Version Upgrade**: Raised the minimum Java version requirement from 11 to 17 for IcebergIO and its dependent modules (SQL extension and examples) to support upcoming Iceberg 1.11.0 requirements.
* **CI Workflow Updates**: Updated five Iceberg-related GitHub Actions workflows to use Java 17 for testing and integration environments.
* **Documentation**: Updated CHANGES.md to reflect the new Java 17 runtime requirement for IcebergIO pipelines and the SQL extension.
<details>
<summary><b>New Features</b></summary>
🧠 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](https://codeassist.google/code-review/login) to enable Memory in your admin console.**
</details>
<details>
<summary><b>Ignored Files</b></summary>
* **Ignored by pattern:** `.github/workflows/**` (5)
* .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.yml
* .github/workflows/IO_Iceberg_Unit_Tests.yml
</details>
<details>
<summary><b>Using Gemini Code Assist</b></summary>
<br>
The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips.
<b>Invoking Gemini</b>
You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page.
Feature | Command | Description
--- | --- | ---
Code Review | `/gemini review` | Performs a code review for the current pull request in its current state.
Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state.
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help | `/gemini help` | Displays a list of available commands.
<b>Customization</b>
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 `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github).
<b>Limitations & Feedback</b>
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with :thumbsup: and :thumbsdown: 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](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG).
</details>
[^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4880584085
Looks like the xlang tests are failing with:
```
2026-07-02T15:21:11.0708685Z [32mINFO [0m ExpansionService:subprocess_server.py:277 Starting expansion service at localhost:58399
2026-07-02T15:21:11.0711989Z [32mINFO [0m ExpansionService:subprocess_server.py:277 Exception in thread "main" java.lang.UnsupportedClassVersionError: org/apache/beam/sdk/io/iceberg/IcebergSchemaTransformTranslation$ReadRegistrar has been compiled by a more recent version of the Java Runtime (class file version 61.0), this version of the Java Runtime only recognizes class file versions up to 55.0
2026-07-02T15:21:11.0715364Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.lang.ClassLoader.defineClass1(Native Method)
2026-07-02T15:21:11.0717070Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1022)
2026-07-02T15:21:11.0719109Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:174)
2026-07-02T15:21:11.0721187Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:800)
2026-07-02T15:21:11.0723651Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:698)
2026-07-02T15:21:11.0725964Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:621)
2026-07-02T15:21:11.0728105Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:579)
2026-07-02T15:21:11.0730204Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
2026-07-02T15:21:11.0732412Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:527)
2026-07-02T15:21:11.0734098Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.lang.Class.forName0(Native Method)
2026-07-02T15:21:11.0735607Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.lang.Class.forName(Class.java:398)
2026-07-02T15:21:11.0737751Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.nextProviderClass(ServiceLoader.java:1210)
2026-07-02T15:21:11.0740073Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNextService(ServiceLoader.java:1221)
2026-07-02T15:21:11.0742426Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNext(ServiceLoader.java:1265)
2026-07-02T15:21:11.0744516Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.util.ServiceLoader$2.hasNext(ServiceLoader.java:1300)
2026-07-02T15:21:11.0746273Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at java.base/java.util.ServiceLoader$3.hasNext(ServiceLoader.java:1385)
2026-07-02T15:21:11.0748596Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at org.apache.beam.sdk.util.construction.PTransformTranslation.loadTransformPayloadTranslators(PTransformTranslation.java:436)
2026-07-02T15:21:11.0751350Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at org.apache.beam.sdk.util.construction.PTransformTranslation.getKnownPayloadTranslators(PTransformTranslation.java:422)
2026-07-02T15:21:11.0754299Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at org.apache.beam.sdk.expansion.service.ExpansionService$ExternalTransformRegistrarLoader.knownTransforms(ExpansionService.java:159)
2026-07-02T15:21:11.0756918Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at org.apache.beam.sdk.expansion.service.ExpansionService.loadRegisteredTransforms(ExpansionService.java:571)
2026-07-02T15:21:11.0759402Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at org.apache.beam.sdk.expansion.service.ExpansionService.getRegisteredTransforms(ExpansionService.java:557)
2026-07-02T15:21:11.0761685Z [32mINFO [0m ExpansionService:subprocess_server.py:277 at org.apache.beam.sdk.expansion.service.ExpansionService.main(ExpansionService.java:846)
2026-07-02T15:21:11.0766598Z [31m[1mERROR [0m apache_beam.utils.subprocess_server:subprocess_server.py:228 Failed to start job service with ['/opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/11.0.31-11/x64/bin/java', '-jar', '/runner/_work/beam/beam/sdks/java/io/expansion-service/build/libs/beam-sdks-java-io-expansion-service-2.76.0-SNAPSHOT.jar', '58399', '--filesToStage=/runner/_work/beam/beam/sdks/java/io/expansion-service/build/libs/beam-sdks-java-io-expansion-service-2.76.0-SNAPSHOT.jar', '--alsoStartLoopbackWorker']
2026-07-02T15:21:11.0770872Z [33mWARNING [0m apache_beam.utils.subprocess_server:subprocess_server.py:242 Error bringing up service (attempt 1 of 3): Service failed to start up with error 1
```
Looks like the jar is getting compiled with Java 17 just fine, but the xlang test's runtime is still using Java 11
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "peterphitran (via GitHub)" <gi...@apache.org>.
peterphitran commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4880740787
Yeah, that matches what I found. Confirmed the jar is Java 17 and it's launched on the runner's Java 11, so ServiceLoader hits the Iceberg registrar and the service fails to start.
I tested it locally, the jar fails on JDK 11 and starts cleanly on JDK 17. The path in the log is $JAVA_HOME/bin/java, so subprocess_server takes the JVM from JAVA_HOME.
On why it's still Java 11, I believe the -PtestJavaVersion=17 we added isn't activating because these are pull_request_target workflows, so CI runs master's YAML, not the PR's. If that's right, the flag only takes effect after merge, which is why the run still shows Java 11.
Worth flagging: since Iceberg is bundled in the shared io expansion-service jar, that jar is effectively Java-17-only at runtime for any Managed transform, not just Iceberg. beam_PerformanceTests_xlang_KafkaIO_Python still defaults to Java 11, so it's likely exposed too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4887780229
I think that makes sense. Can we go through and pin Java 17 on all test suites that use IO expansion service?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "peterphitran (via GitHub)" <gi...@apache.org>.
peterphitran commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4786288057
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #39064:
URL: https://github.com/apache/beam/pull/39064#discussion_r3455508805
##########
CHANGES.md:
##########
@@ -2473,4 +2474,4 @@ Schema Options, it will be removed in version `2.23.0`. ([BEAM-9704](https://iss
## 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/).
Review 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.
```suggestion
- For versions 2.19.0 and older release notes are available on [Apache Beam Blog](https://beam.apache.org/blog/).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on code in PR #39064:
URL: https://github.com/apache/beam/pull/39064#discussion_r3462702680
##########
sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/CreateReadTasksDoFn.java:
##########
Review Comment:
unnecessary change
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -2715,6 +2715,12 @@ class BeamModulePlugin implements Plugin<Project> {
def usesDataflowRunner = config.pythonPipelineOptions.contains("--runner=TestDataflowRunner") || config.pythonPipelineOptions.contains("--runner=DataflowRunner")
String ver = project.findProperty('testJavaVersion')
def javaContainerSuffix = ver ? getSupportedJavaVersion(ver) : getSupportedJavaVersion()
+ // Resolve the JDK used to launch the Python-started expansion service (see the exec block
+ // below). Prefer the explicitly requested test JDK (-PtestJavaVersion); otherwise fall back
+ // 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')
Review Comment:
should this be `project.findProperty("java${ver}Home")` ?
##########
sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/AddFiles.java:
##########
Review Comment:
unnecessary change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4855277523
Reminder, please take a look at this pr: @kennknowles @Abacn
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-5045895943
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @ahmedabu98 for label java.
R: @damccorm for label build.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4958206275
Reminder, please take a look at this pr: @kennknowles @Abacn @ahmedabu98
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-5003342624
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @ahmedabu98 for label java.
R: @liferoad for label build.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4775689592
Assigning reviewers:
R: @kennknowles for label java.
R: @Abacn for label build.
Note: If you would like to opt out of this review, comment `assign to next reviewer`.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4775102871
## [Codecov](https://app.codecov.io/gh/apache/beam/pull/39064?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 57.73%. Comparing base ([`eaba570`](https://app.codecov.io/gh/apache/beam/commit/eaba570c5697f7eea60d1467d7ac18e08ca23d15?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f264257`](https://app.codecov.io/gh/apache/beam/commit/f264257aeb6fd3b82d3a8321429f6813c700d3f0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 4 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #39064 +/- ##
============================================
+ Coverage 54.24% 57.73% +3.48%
- Complexity 1715 13051 +11336
============================================
Files 1064 2513 +1449
Lines 166935 261551 +94616
Branches 1255 10740 +9485
============================================
+ Hits 90562 151002 +60440
- Misses 74158 104815 +30657
- Partials 2215 5734 +3519
```
| [Flag](https://app.codecov.io/gh/apache/beam/pull/39064/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [java](https://app.codecov.io/gh/apache/beam/pull/39064/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `64.21% <ø> (-3.40%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/beam/pull/39064?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #39064:
URL: https://github.com/apache/beam/pull/39064#discussion_r3603532667
##########
.github/workflows/IO_Iceberg_Managed_Integration_Tests_Dataflow.yml:
##########
@@ -74,6 +74,8 @@ jobs:
github_job: ${{ matrix.job_name }} (${{ matrix.job_phrase }})
- name: Setup environment
uses: ./.github/actions/setup-environment-action
+ with:
Review Comment:
We can revert all these Java version overrides now in .github/workflows. CI now defaults to Java21 runtime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-5123418057
Merging now, but let's keep an eye on relevant workflows in case they start breaking due to this Java bump
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "peterphitran (via GitHub)" <gi...@apache.org>.
peterphitran commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4784338901
@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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #39064:
URL: https://github.com/apache/beam/pull/39064#discussion_r3463663283
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -2715,6 +2715,12 @@ class BeamModulePlugin implements Plugin<Project> {
def usesDataflowRunner = config.pythonPipelineOptions.contains("--runner=TestDataflowRunner") || config.pythonPipelineOptions.contains("--runner=DataflowRunner")
String ver = project.findProperty('testJavaVersion')
def javaContainerSuffix = ver ? getSupportedJavaVersion(ver) : getSupportedJavaVersion()
+ // Resolve the JDK used to launch the Python-started expansion service (see the exec block
+ // below). Prefer the explicitly requested test JDK (-PtestJavaVersion); otherwise fall back
+ // 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')
Review 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #39064:
URL: https://github.com/apache/beam/pull/39064#discussion_r3463663283
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -2715,6 +2715,12 @@ class BeamModulePlugin implements Plugin<Project> {
def usesDataflowRunner = config.pythonPipelineOptions.contains("--runner=TestDataflowRunner") || config.pythonPipelineOptions.contains("--runner=DataflowRunner")
String ver = project.findProperty('testJavaVersion')
def javaContainerSuffix = ver ? getSupportedJavaVersion(ver) : getSupportedJavaVersion()
+ // Resolve the JDK used to launch the Python-started expansion service (see the exec block
+ // below). Prefer the explicitly requested test JDK (-PtestJavaVersion); otherwise fall back
+ // 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')
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IcebergIO] Raise Java 17 floor for IcebergIO's Java 11 dependents [beam]
Posted by "peterphitran (via GitHub)" <gi...@apache.org>.
peterphitran commented on PR #39064:
URL: https://github.com/apache/beam/pull/39064#issuecomment-4888178835
Pinned java-version: '17' on every suite that launches the IO expansion service:
- beam_PostCommit_Python_Xlang_IO_Direct
- beam_PostCommit_Python_Xlang_IO_Dataflow
- beam_PreCommit_Xlang_Generated_Transforms
- beam_PerformanceTests_xlang_KafkaIO_Python
(build_release_candidate already builds on '17'.)
With only Java 17 installed, the default JAVA_HOME is 17, so the Python subprocess launches the jar on 17. This matches how master already pins the wrapper-validation precommit. Since the pin handles the launch JDK directly, I also dropped the -PtestJavaVersion/-Pjava17Home flags and the now-unused JAVA_HOME redirect in createCrossLanguageUsingJavaExpansionTask, so there's a single mechanism.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org