SEZ9 commented on PR #11545: URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-6051036904
Thanks for the corrections and the update against `74ac922d6`. Going through the open points: **F2 (Kerberos reload detection)** – Thanks for clarifying; if `KuduUtil` and `PaimonSecurityContext` log an ERROR naming the missing `--add-exports` flag when the reflective call is denied, that covers the silent-failure concern. I have not yet looked at `1b1da9e86` myself, so I will confirm against the diff before closing this one. **F1 / F5 (preserved `config/` directory, JDK 8 leftover)** – The launcher behaviour you describe (Java version check, then appending each module flag only if not already present) is the right shape for both the config-upgrade gap and the cryptic `Unrecognized option` abort. I will verify the `seatunnel.sh` / `seatunnel-cluster.sh` blocks in the diff. One ask: please add a short note in the PR description or in the config comments that the shipped `config/jvm_*_options` flags are a convenience and the launcher is the source of truth, so nobody later removes the launcher injection thinking the config files are sufficient. **F6 / F8 (`JAVA_TOOL_OPTIONS` in CI)** – Dropping the jgss export from the other workflows in `e936b6dcc` is helpful, but F6/F8 are about `backend.yml`. As I understand it, `45a14d047` only updates the comment there and the repeated flags remain. I can accept that as a temporary state if the comment links the blocking connector issue; please open a follow-up to remove those flags once that issue is fixed, so CI again proves the build on a plain JDK. **F3 (minimality of the six flags)** – Still open. Since only `java.net` has a captured failing trace, please either (a) capture the failing trace for each of the other five opens/exports, or (b) explicitly mark the unproven ones in the PR description and the config comments so they can be trimmed later. Either is fine; I just don't want them to look authoritative without evidence. **F4 / F7 (2.3.13 in `upgrade_compatibility.yml`)** – Still open. You mention the workflow runs the old release on JDK 11, which would make my JDK 17 concern moot, but I'd like to confirm that from the workflow itself: please point me at the step or matrix entry that pins the old-cluster JDK, and confirm the old cluster starts cleanly now that the jgss export is removed from that workflow. If so, F4/F7 can close. **CI** – Run 37331927562 at `d1794e8fb` finishing with `87 success, 9 skipped` is good evidence for the code changes. Since `d1794e8fb..74ac922d6` only touches workflows, comments and the `dev` merge, I'd still like to see the run for `74ac922d6` finish before merging; please post the result when it lands. <!-- streview-comment:1589 --> -- 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]
