SEZ9 commented on PR #11545: URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-5852135344
Thanks for the detailed pointers. A few notes before I can mark anything resolved: - **F5, F1 (launcher side), F2, F4/F7** – Your description of the launcher pre-check, the idempotent flag-append loop, the ERROR logging on `IllegalAccessException`, and running the old 2.3.13 cluster on JDK 11 sounds like the right shape for each. However, you also note that the only change between `2c87f42f2b236` and `50223b3014626` touched the two unit-test steps in `backend.yml`, so I can't see those changes in this round's diff. Could you confirm they are pushed on this PR (and point me at the commit) so I can verify against the actual diff rather than the description? - **F1 (docs)** – Understood that the `incompatible-changes.md` note isn't committed yet; happy for it to land together with your follow-up fix rather than as a doc-only commit. - **F3** – Still open: the blanket `--add-opens`/`--add-exports` to `ALL-UNNAMED` in `config/jvm_master_options` and siblings. Could you either narrow the list to what the affected components need, or add a short comment per flag so operators can prune safely? - **F6 / F8** – Still open: the workflow-level `JAVA_TOOL_OPTIONS` in `backend.yml`. Now that the six flags are repeated directly in `SUREFIRE_JVM_ARGS` for the unit-test steps, is the global env still needed? If not, please drop it so CI runs on a plain JDK 11/17 and the "Picked up JAVA_TOOL_OPTIONS" stderr noise goes away; if it is, a note on which job depends on it would help. Once the fixes above are pushed, along with the other items you flagged as still open in your table, I'll take another pass. <!-- streview-comment:1357 --> -- 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]
