DanielLeens commented on PR #11545: URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-6033665848
Thanks @SEZ9. Answering against head `d1794e8fb` (still the PR head as of this comment). **Attempt 2 of run 37331927562 finished green.** The whole run concluded `success`: the four jobs I re-ran with `--failed` (`benchmark-test (11)`, `all-connectors-it-2 (11)`, `edge-agent-it (17)`, `all-connectors-it-6 (17)`) passed on the second attempt, and the unit-test legs passed on both attempts. None of the four failed twice, so there is nothing to root-cause separately. One caveat: this is a fork run, and I am reporting it as evidence for this head only; the apache-side `Build` check is the gate. **F6 / F8 hunks to check against the code** - Launcher scripts: `seatunnel-core/seatunnel-starter/src/main/bin/seatunnel.sh` lines 112-131 and `seatunnel-cluster.sh` lines 157-176 (Java version check, then a loop that appends each of the six module flags only when the effective `jvm_*_options` does not already contain it). - Test JVMs: root `pom.xml` line 186 defines `surefire.module.args`; lines 798 and 830 feed it into the surefire and failsafe `argLine`. - The `JAVA_TOOL_OPTIONS` removal from the test lanes is in `.github/workflows/backend.yml`. **JAVA_TOOL_OPTIONS in the three other workflows.** I will do the small separate commit you described (drop only the jgss export, keep the heap flags in `codeql.yaml`, state in the commit message that `publish-docker.yaml` is unverified). I have not pushed it yet, to avoid cancelling checks that are still being evaluated; it goes in with the next push, together with the issue link in the PR description. **Remaining findings, short answers** - **F1**: covered by the launcher scripts above, so a preserved `config/` directory still gets the flags, and the en/zh `incompatible-changes.md` entry (item 5) says so. - **F2**: no runtime detection or log line was added in this PR. I have not done that work; it is deferred and I will say so explicitly in the description rather than imply coverage. - **F3**: each flag in `config/jvm_options` (and the three sibling files) is now annotated with the component that needs it. The list is not proven minimal. Only `java.net` has a captured failing stack trace (`ReflectionUtilsTest`); the Hazelcast-attributed flags are unbisected. - **F4 / F7**: `upgrade_compatibility.yml` pins the old 2.3.13 cluster to JDK 11, not 17, so I have not shown it starting on 17, and I have not run it with the jgss export removed. Both are untested. - **F5**: done, the Java version pre-check with the `SeaTunnel requires Java 11 or newer` message is in both launcher scripts (lines above). -- 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]
