SEZ9 commented on PR #11718: URL: https://github.com/apache/seatunnel/pull/11718#issuecomment-5421605718
Thanks for confirming, @DanielLeens — and for re-checking that the head is still `d73c2b90bf`. To answer directly: the follow-up commit is not up yet, so there's nothing for you to look at right now. What's left on my side, all scoped to `PaimonWithS3IT.java` as agreed: 1. **Item 1 (in-container kill):** adding the best-effort kill of the wedged `seatunnel.sh` client on the timeout path. I'm keeping your two review criteria in mind while writing it — it must stay non-blocking on the timeout path, and it must not disturb the `runningCount`/thread-leak-check interaction already documented in the class. If it starts getting involved, I'll take you up on splitting it into its own PR per your earlier suggestion. 2. **Item 2 (Javadoc scoping):** narrowing the bound's Javadoc so it only claims coverage of the single-arg `executeJob(String)` overload, rather than promising all future submissions are bounded. 3. **Item 6 (local-run caveat):** adding the caveat sentence about the non-daemon junit-timeout worker holding the JVM open in local/IDE runs where Failsafe's ForkedBooter halt doesn't apply. Agreed on the process: this stays a single follow-up commit on this PR, and I'll ping this thread as soon as it's pushed so you can do the focused pass on the in-container-kill piece and the quick verification of the two doc fixes against the call sites. Nothing needed from you until then. <!-- streview-comment:575 --> -- 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]
