SEZ9 commented on PR #11545: URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-5650650805
Thanks for the follow-up on `67d5410bfb`. **F6/F8** — Moving the flags from a workflow-level `JAVA_TOOL_OPTIONS` in `.github/workflows/backend.yml` into a JDK-activated `jdk9-plus-test-opens` profile that feeds `surefire.jvm.args` is the right shape: the flags now live where the forked test JVMs are actually created, local `./mvnw test`/`verify` matches CI, and the `Picked up JAVA_TOOL_OPTIONS` noise goes away. Two small asks before I mark these closed: - Please confirm the profile is applied to Failsafe as well as Surefire (the description says both, I just want to see it in the pom). - Please confirm no other workflow or module still sets `JAVA_TOOL_OPTIONS` with these flags, so we don't end up with two sources of truth. **F3** — Per-flag attribution comments in `config/jvm_*_options` are an acceptable resolution for this PR, and I agree with not trimming flags here: verifying Hazelcast's internal reflective usage is out of scope and the downside of a wrong guess is a silent runtime failure. The `java.net` (`URLClassLoader#addURL` in `AbstractPluginDiscovery`, `ConfigValidationUtils`, Flink starter loaders) and krb5 (`KuduUtil`, `PaimonSecurityContext`) attributions are concrete and useful. For the `java.lang`/`java.nio`/`java.util`/`sun.nio.ch` flags, saying "required by Hazelcast internals" is honest; please don't phrase it more precisely than that. Consider F3 closed once I've seen the comments in the diff. **Still open from the previous review:** F1 (module flags only in user-overridable `config/jvm_*_options`, so preserved config dirs on upgrade drop them), F2 (no runtime detection when the Kerberos reflective reload fails due to missing flags), F4/F7 (`upgrade_compatibility.yml` running the old 2.3.13 release on JDK 17 with only the jgss export), and F5 (no graceful Java-version pre-check before passing `--add-opens` on a JDK 8 runtime). Nothing in this update touches those; please either address them or state explicitly which ones you'd like to defer to a follow-up and why, so we can agree on scope. Once the build on the new head finishes, please post the result here. <!-- streview-comment:1010 --> -- 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]
