DanielLeens commented on PR #12298: URL: https://github.com/apache/seatunnel/pull/12298#issuecomment-5696104922
Thanks for the detailed CI write-up, @SEZ9. I independently pulled the fork's run for this head (`SEZ9/seatunnel` run `34993525332`, head `d671c5d86787eeccc60e4c37df325257f4cb8ce5` — same head I reviewed) rather than taking the summary on trust, and can confirm it matches what you reported: - `engine-v2-it` and `unit-test` are green on every leg (8/11, ubuntu/windows where applicable) — this PR's own change and its test (`testDynamicHttpPortIsResolvableByPeers`) pass. - The only two red jobs are exactly `all-connectors-it-2` (8 and 11) and `all-connectors-it-1` (11), and the surefire output confirms the failing tests are what you said: `OpengaussCDCIT.testAddFieldWithRestore:476` (`ConditionTimeout`, `Tests run: 15/23, Errors: 1`) and `NebulaGraphIT.startUp:109` (`expected: <true> but was: <false>`). Neither test file, nor anything in the CDC/NebulaGraph modules, is touched by this diff. - Cross-checked `OpengaussCDCIT` against #12299's fork run (`34993534806`) as you suggested — same test, same line, same `ConditionTimeoutException` on an unrelated PR that only touches REST log endpoints, not CDC. That's solid corroboration it's pre-existing/environmental rather than something this diff triggers. One correction on the `NebulaGraphIT` hypothesis, since I want the record here to be accurate rather than just deferring to the write-up: I don't think the testcontainers 1.21.4 bump (#11201) is the actual trigger. #12329 (open, not yet merged) documents this exact `startUp()` failure — same class, same `graphd` status-vs-RPC-port race — already occurring on 2026-09-13 against #12081 and #11727, and #11201 didn't merge until 2026-09-15T13:42:39Z, roughly two days later. So the race predates the version bump and is already tracked with a fix in flight; it isn't new exposure from the rebase. Doesn't change your conclusion at all — if anything it's stronger evidence this is unrelated to #12298 — just flagging so nobody chases the testcontainers angle as the root cause elsewhere. To close the loop on process: the one open item I was tracking (the `incompatible-changes.md` entry) was already withdrawn in my last review after your rebuttal — `HttpConfig.getPort()` is an internal accessor with no published config/API surface, and the only observable effect is a previously-broken fan-out endpoint now working, so there's no migration action for an upgrading user to document. Nothing new to reopen there. No source-level objections from me. This PR's own tests are green, and the two remaining red jobs are confirmed pre-existing failures unrelated to this change (one independently corroborated against #12299, the other against a separately-tracked, already-open fix in #12329). I'd treat this as ready to merge once a maintainer with merge rights takes it — this account is comment-only. -- 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]
