SEZ9 commented on PR #11545: URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-6029496735
Thanks for the detailed update on head `d1794e8fb` and run 37331927562. **F6 / F8 (`JAVA_TOOL_OPTIONS`)** - You note it is no longer set in `backend.yml` and that the flags now live in the shipped config, launcher scripts and test JVMs, with the step comments in `c284df271`. Could you point me to the diff hunks for the launcher-script and test-JVM wiring so I can mark F6 resolved against the code rather than the description? - For the remaining single jgss `--add-exports` in `codeql.yaml`, `publish-docker.yaml` and `upgrade_compatibility.yml`: since you believe there is no remaining consumer after the reflection changes, a small separate commit removing it works for me, with the commit message stating that `publish-docker.yaml` is unverified. In `codeql.yaml`, please keep the heap flags and drop only the module export. If you would rather keep it anywhere, please name the concrete consumer. **Run 37331927562** - Understood that the four unit-test legs and `Dependency licenses` passed, that the four failing jobs show no module-access errors, and that attempt 2 is queued. Please post the attempt-2 outcome here; for anything that fails twice, a separate root-cause rather than another retry sounds right. **Remaining findings** — a short reply per item would help, even if it is "won't fix, because …": - **F1**: with the flags in user-overridable `config/jvm_*_options`, an upgrade that preserves an existing `config/` directory may drop them. Does the launcher-script wiring you mention cover that case, or is a release note / startup check planned? - **F2**: is there now runtime detection or a log line when the Kerberos reflective reload runs without the flags, or is that deferred to the follow-up issue? If deferred, please link the issue in the PR description with the next push, as you planned. - **F3**: please confirm the `--add-opens/--add-exports` list to `ALL-UNNAMED` is the minimal set needed, and annotate in the config files which component requires each entry. - **F4 / F7**: in `upgrade_compatibility.yml`, does the old 2.3.13 cluster start successfully on JDK 17 in the current run, and does it still start if the jgss export is removed per F8? - **F5**: a Java-version pre-check in the launcher scripts with a clear message would avoid the `Unrecognized option` abort on a leftover JDK 8. Once the attempt-2 results and the above are in, I will do a final pass. <!-- streview-comment:1567 --> -- 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]
