SEZ9 commented on PR #11814: URL: https://github.com/apache/seatunnel/pull/11814#issuecomment-5409350994
Thanks @DanielLeens for the thorough re-review and for re-tracing the wiring on `a67ad2a93460eabf8f9e672b14fed6a0d926b126` rather than taking the change on faith — much appreciated. To confirm your reading of the delta: the only change since `8231fd293f24` is the +6/-0 commit to `FakeSourceToConsoleWithEventReportIT.java`, giving the container an explicit 3-minute startup wait in place of the base class's default 60s `Wait.forLogMessage(...)`. This directly follows the diagnostic step you proposed last round for the ambiguous E2E timeout, and it's good to hear the wait strategy is now correctly wired to actually gate readiness — that was the crux of the original finding on this file. All other items from the earlier rounds (the `close()`/scheduler shutdown hardening, exception handling in the flush path, the okhttp3 4.12.0 migration and its dependency/license registrations, the retry-path regression test, and the redirect/Content-Type behavior questions) remain as fixed in the previous commits, with no changes this round — consistent with your diff stat. On your one remaining ask: agreed that the change is well-reasoned but not yet confirmed until CI completes on this exact head. I'll make sure a full run goes through on `a67ad2a93460eabf8f9e672b14fed6a0d926b126` and post back here once it finishes, with particular attention to whether the event-report E2E test passes reliably under the extended wait. If it does, I'd ask you to take one final look to close out the review; if the timeout still reproduces, we'll know the wait duration wasn't the root cause and can dig further. Thanks again for the careful, multi-round review — it made this migration much safer. <!-- streview-comment:561 --> -- 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]
