DanielLeens commented on PR #11814:
URL: https://github.com/apache/seatunnel/pull/11814#issuecomment-5662012668
@SEZ9 — I think this crossed with my comment from about 11 minutes earlier
on this same thread, where I went through F1-F8 individually against this exact
head (`d9855aee7d7`) and found all eight already resolved, each with a specific
line pointer. Restating them here directly so there's no ambiguity about what's
already on the branch:
- **F1 (wait-strategy ordering)** — `SeaTunnelContainer.java`:
`createSeaTunnelServer(Network)` calls `executeExtraCommands(server)` at line
147 and only calls `server.start()` afterward at line 149.
`FakeSourceToConsoleWithEventReportIT.startUp()` resolves through that exact
overload, and its `executeExtraCommands` override installs the
`LogMessageWaitStrategy` there — before `start()` runs, so it does reliably
gate readiness on this test's actual call path.
- **F2/F3 (`close()` hardening)** —
`JobEventHttpReportHandler.java:213-259`: both final flushes are gated on
`schedulerTerminated && !interrupted` (`:237`) with a logged `else` branch
instead of racing an interrupted/still-running scheduler; both flushes are
wrapped in `catch (Exception e)` (`:241`, `:246`), not the old narrow
`HazelcastInstanceNotActiveException`/`IOException` pair;
`dispatcher().cancelAll()` runs before `connectionPool().evictAll()` in the
outer `finally` (`:253-254`), so in-flight calls are cancelled before their
connections are evicted.
- **F4/F6 (Kotlin stdlib split)** — `known-dependencies.txt` lists
`kotlin-stdlib{,-common,-jdk7,-jdk8}` all at `1.9.10` (no `1.8.21` remaining);
`seatunnel-engine-server/pom.xml:30-37` imports `kotlin-bom:1.9.10` in
`dependencyManagement`, pinning the whole graph instead of leaving it to Maven
nearest-wins.
- **F5 (`testRetryAfterHttpFailure` hygiene)** —
`JobEventHttpReportHandlerTest.java:208-209`: both `takeRequest` calls use an
explicit `10, TimeUnit.SECONDS` bound; the redirect test at `:247`/`:252`
follows the same pattern.
- **F7 (redirect-following)** — `JobEventHttpReportHandler.java:264-275`:
`createHttpClient()` builds with
`.followRedirects(false).followSslRedirects(false)`, so a 3xx surfaces through
the existing non-2xx/retry/log path instead of forwarding configured report
headers to a redirect target on another host.
- **F8 (Content-Type charset)** — `JobEventHttpReportHandlerTest.java:175`:
`testReportEvent` asserts the emitted header is exactly `"application/json;
charset=utf-8"`, locking the migration's behavior in by assertion rather than
just review claim.
I also independently verified the merge itself changed none of this: `git
diff af713e463f3..d9855aee7d7 -- <every file this PR's diff touches>` is empty
for the handler, its test, and all three POMs — the only PR-owned files the
merge commit touches are the `incompatible-changes.md` docs (en+zh), where the
resolution is pure content-preservation (three unrelated `dev` entries appended
alongside this PR's own, nothing removed or altered). So this really is a
mechanical sync, and F1-F8 were already fixed on `af713e463f3` before it landed
— same conclusion you reached independently in your comment just above.
If you're seeing something different on your end for any of these (e.g. a
different line range or a case I'm not tracing correctly), point me at the
specific spot and I'll re-check it directly — but from current source I don't
have anything left open here.
On CI: as of this comment the fork's `Build` run for `d9855aee7d7` is still
queued/in-progress, so I don't have a completed result on this exact merge
commit yet. Given the merge is a no-op on every file this PR touches, I'd
expect it to match the previous head's result (both `engine-v2-it` legs green,
with the `minio/minio` registry failures elsewhere being an unrelated,
independently-observed outage that night) — but that's an expectation, not a
confirmed result, so please don't treat this as a green signal until the run
actually completes.
--
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]
