SEZ9 commented on PR #11718: URL: https://github.com/apache/seatunnel/pull/11718#issuecomment-5409364752
Thanks @DanielLeens — that settles the open question, and I agree with your reasoning. I'll go with a follow-up commit on this same PR rather than a separate one. Since Items 1, 2, and 6 all live in `PaimonWithS3IT.java`, which is already under review here, keeping them together avoids rebuilding the verification context a second time. Concretely, the follow-up commit will cover: - **Item 1** (timeout-path cleanup): add a best-effort in-container kill of the wedged `seatunnel.sh` client on the timeout path, so it doesn't suppress the thread-leak check or leave later tests in the PER_CLASS instance running against a wedged cluster. - **Item 2** (Javadoc scoping): tighten the wording so it accurately says the bound covers the single-arg `executeJob(String)` overload only, rather than promising automatic coverage for any future submission path. - **Item 6** (local/IDE caveat): add the caveat sentence about the non-daemon junit-timeout worker holding the JVM open outside Failsafe's ForkedBooter halt. None of this is pushed yet — the current head is still `d73c2b90bf`, which as you said is approved and mergeable as-is. What I'd ask from you once the follow-up commit lands: a quick pass on the in-container-kill piece specifically. If it turns out to need real back-and-forth, I'm happy to split it into its own PR at that point per your suggestion, and land just the two documentation fixes here. I'll ping this thread when the commit is up. <!-- streview-comment:535 --> -- 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]
