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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
