DanielLeens commented on PR #11545: URL: https://github.com/apache/seatunnel/pull/11545#issuecomment-6027085464
Thanks @SEZ9. Going through your list against head `d1794e8fb` and fork run 37331927562 (now complete): **1. `unit-test (17)` result.** In run 37331927562 all four unit-test legs passed: `unit-test (11)` and `unit-test (17)` on ubuntu and on windows, and `Dependency licenses` passed too. The run as a whole is still red, so I am not calling it green: four other jobs failed (`benchmark-test (11)`, `all-connectors-it-2 (11)` cancelled at its 2h30m timeout after a silent hang in `PulsarMultiTableIT`, `edge-agent-it (17)` in `SavePointTest.testSavePointButJobGoingToFail`, and `all-connectors-it-6 (17)` in `DatabendCDCSinkIT` with `expected: <3> but was: <0>`). I read each log; none shows a module-access error, the test files are unchanged from the Aug 28 green head, and the other JDK leg of the same job passed for the first three. That is circumstantial, not proof, so I re-ran all four with `--failed` (attempt 2 of the same run, queued). I will report that result, and any job that fails a second time I will root-cause separately instead of retrying again. **2. Description and comments.** The PR description is updated: it no longer says the flags reach CI via `JAVA_TOOL_OPTIONS`, it lists the three places the flags now live (shipped config, launcher scripts, test JVMs), and it has an explicit paragraph that the unit-test lane intentionally repeats the flags, why, and that only the integration-test lanes rely on `surefire.module.args` alone. The step comments in `backend.yml` already say this (`c284df271`). I have not pushed to add a reference to the follow-up issue there, because a push would cancel the re-run that is now in progress; I will add it with the next push. **3. `JAVA_TOOL_OPTIONS`.** It is no longer set in `backend.yml` on this head. It is still set, as a single `--add-exports=java.security.jgss/sun.security.krb5=ALL-UNNAMED`, in three non-test workflows: `codeql.yaml` (which also puts heap flags in the same variable), `publish-docker.yaml` and `upgrade_compatibility.yml`. So the F6 concern (CI never running on a plain JDK 11/17) does not apply to the test lanes any more. The stderr noise from F8 can still appear in those three. I believe the jgss export has no remaining consumer there, since `KuduUtil` and `PaimonSecurityContext` use reflection now, but I have not proven it, and `publish-docker.yaml` cannot be exercised cheaply. If you want them removed I will do it as a separate small commit and note that it is unverified until those workflows run. **4. Follow-up for `connector-lance`.** Filed as https://github.com/apache/seatunnel/issues/12655, with the evidence, the cause (its own `<argLine>` and `surefire.jvm.args`), and the suggested change (inherit the root argLine, then drop the repeated flags from `SUREFIRE_JVM_ARGS`). It is a `pom.xml` change, so I have left it out of this PR. -- 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]
