gemini-code-assist[bot] commented on code in PR #38478:
URL: https://github.com/apache/beam/pull/38478#discussion_r3229642077
##########
runners/spark/4/build.gradle:
##########
@@ -36,13 +36,23 @@ apply from: "$basePath/spark_runner.gradle"
// Spark 4 always requires Java 17, so unconditionally add the --add-opens
flags
// required by Kryo and other libraries that use reflection on JDK internals.
-test {
+tasks.withType(Test).configureEach {
jvmArgs "--add-opens=java.base/sun.nio.ch=ALL-UNNAMED",
"--add-opens=java.base/java.nio=ALL-UNNAMED",
"--add-opens=java.base/java.util=ALL-UNNAMED",
"--add-opens=java.base/java.lang.invoke=ALL-UNNAMED"
}
Review Comment:
Spark 4 on Java 17 typically requires a more comprehensive set of
`--add-opens` flags to handle internal reflection used by Spark's memory
management and serialization (e.g., Kryo). Missing flags like `java.lang`,
`java.io`, and `java.net` can lead to `IllegalAccessException` in certain
workloads.
```
tasks.withType(Test).configureEach {
jvmArgs "--add-opens=java.base/sun.nio.ch=ALL-UNNAMED",
"--add-opens=java.base/java.nio=ALL-UNNAMED",
"--add-opens=java.base/java.util=ALL-UNNAMED",
"--add-opens=java.base/java.lang=ALL-UNNAMED",
"--add-opens=java.base/java.lang.invoke=ALL-UNNAMED",
"--add-opens=java.base/java.io=ALL-UNNAMED",
"--add-opens=java.base/java.net=ALL-UNNAMED",
"--add-opens=java.base/java.util.concurrent=ALL-UNNAMED"
}
```
##########
runners/spark/4/build.gradle:
##########
@@ -36,13 +36,23 @@ apply from: "$basePath/spark_runner.gradle"
// Spark 4 always requires Java 17, so unconditionally add the --add-opens
flags
// required by Kryo and other libraries that use reflection on JDK internals.
-test {
+tasks.withType(Test).configureEach {
jvmArgs "--add-opens=java.base/sun.nio.ch=ALL-UNNAMED",
"--add-opens=java.base/java.nio=ALL-UNNAMED",
"--add-opens=java.base/java.util=ALL-UNNAMED",
"--add-opens=java.base/java.lang.invoke=ALL-UNNAMED"
}
+tasks.validatesStructuredStreamingRunnerBatch {
+ filter {
+ // TODO(https://github.com/apache/beam/issues/36841): currently failing
with INTERNAL_ERROR]
+ // Couldn't find method fromByteArray with arguments
+ excludeTestsMatching
'org.apache.beam.sdk.transforms.CombineTest$BasicTests.testHotKeyCombining'
+ }
+}
Review Comment:
To support Gradle configuration avoidance, it is recommended to use
`tasks.named(...).configure { ... }` instead of eager task configuration.
Additionally, fixed a minor typo in the TODO comment (missing opening bracket
for `[INTERNAL_ERROR]`).
```
tasks.named("validatesStructuredStreamingRunnerBatch").configure {
filter {
// TODO(https://github.com/apache/beam/issues/36841): currently failing
with [INTERNAL_ERROR]
// Couldn't find method fromByteArray with arguments
excludeTestsMatching
'org.apache.beam.sdk.transforms.CombineTest$BasicTests.testHotKeyCombining'
}
}
```
##########
runners/spark/4/build.gradle:
##########
@@ -36,13 +36,23 @@ apply from: "$basePath/spark_runner.gradle"
// Spark 4 always requires Java 17, so unconditionally add the --add-opens
flags
// required by Kryo and other libraries that use reflection on JDK internals.
-test {
+tasks.withType(Test).configureEach {
jvmArgs "--add-opens=java.base/sun.nio.ch=ALL-UNNAMED",
"--add-opens=java.base/java.nio=ALL-UNNAMED",
"--add-opens=java.base/java.util=ALL-UNNAMED",
"--add-opens=java.base/java.lang.invoke=ALL-UNNAMED"
}
+tasks.validatesStructuredStreamingRunnerBatch {
+ filter {
+ // TODO(https://github.com/apache/beam/issues/36841): currently failing
with INTERNAL_ERROR]
+ // Couldn't find method fromByteArray with arguments
+ excludeTestsMatching
'org.apache.beam.sdk.transforms.CombineTest$BasicTests.testHotKeyCombining'
+ }
+}
+
+tasks.validatesRunner.dependsOn(validatesStructuredStreamingRunnerBatch)
Review Comment:
The PR description mentions adding "portable validates runner tests", but
they are not included as a dependency for the `validatesRunner` task. If
`validatesPortableRunnerBatch` was intended to be part of this PR, it should be
added here. Also, using `tasks.named` and string references for dependencies
supports configuration avoidance.
```
tasks.named("validatesRunner").configure {
dependsOn "validatesStructuredStreamingRunnerBatch"
}
```
--
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]
