SEZ9 commented on PR #11814: URL: https://github.com/apache/seatunnel/pull/11814#issuecomment-5658380388
Thanks for syncing `dev` into the branch. Comparing `af713e463f3..d9855aee7d7`, the merge commit `d9855aee7` does not change the handler, its unit test, or the module POMs this PR touches; the only PR-owned files affected are the incompatible-changes docs, where the resolution just appends the unrelated `dev` entries alongside this PR's own entries. So this looks like a purely mechanical sync. That also means the previously open findings are unchanged. To close them out I still need the following on the branch (or a reason why a given one should stay as-is): - **F1 (HIGH, E2E wait strategy)** – move the `received new worker register` wait out of `executeExtraCommands` and into the container lifecycle configuration so it actually gates server readiness before the test runs. - **F2 (MEDIUM, close() vs. running task)** – when the scheduler does not terminate in time, `close()` should not flush the local buffer concurrently with the still-running task; please serialize the flush with the task and handle in-flight connections that `evictAll()` cannot reclaim. - **F3 (MEDIUM, close() exception handling)** – the blocking Hazelcast/HTTP flush in `close()` may run on an interrupted thread and currently only catches `HazelcastInstanceNotActiveException`/`IOException`; other runtime failures still escape `close()`. - **F4 / F6 (MEDIUM/LOW, Kotlin stdlib versions)** – compile-scope okhttp3 brings `kotlin-stdlib` 1.8.21 and `kotlin-stdlib-common` 1.9.10 onto the server classpath, and the same split shows up in the dependency and license inventories. Please either pin the Kotlin stdlib artifacts to one version or confirm the resolved dist actually ships this mix and the inventories match it. - **F5 (MEDIUM, `testRetryAfterHttpFailure`)** – use `takeRequest` with a timeout so the build cannot hang, close the request `Buffer`s, and don't rely on the scheduler firing immediately given the 1-day interval (trigger the flush explicitly or shorten the interval in the test). - **F7 (LOW, redirects)** – disable redirect following on the new `OkHttpClient` (or strip the configured headers on cross-host redirects) so report headers carrying tokens are not forwarded to another host. - **F8 (LOW, Content-Type)** – `RequestBody.create(String, MediaType)` appends `; charset=utf-8` when the configured media type has no charset; please confirm whether the header sent to existing collectors stays byte-identical, and if not, build the body from bytes with the original `MediaType`. If any of these were already handled in a commit I have not seen, just point me to it and I will re-check. Otherwise, once the above land I'm happy to do one more pass. <!-- streview-comment:1032 --> -- 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]
