DanielLeens commented on PR #12218: URL: https://github.com/apache/seatunnel/pull/12218#issuecomment-5627581784
Thanks @Rangsh — I independently pulled the fork run you referenced (`Rangsh/seatunnel` run `34433792914`) rather than taking the summary at face value: it's `completed`/`success`, 79 success + 14 skipped + 0 failure, and I confirmed by job name that `engine-v2-it (8, ubuntu-latest)`, `engine-v2-it (11, ubuntu-latest)`, `connector-file-local-it (8, ubuntu-latest)`, and `connector-file-local-it (11, ubuntu-latest)` are all `success` — exactly the jobs I asked to see finish green in my last review before treating the CI condition as satisfied. That condition is now met. Combined with the code-level findings from my last two rounds — Issue 1 (partial-submission orphaned-task/classloader-leak risk) resolved and verified by a genuine regression test in `258a779d7`, and Issue 2 (`CooperativeTaskWorker` asymmetry) resolved and traced correct in `16111de97` — I don't have any remaining blocking findings on this PR. Head is still `16111de97e`, so there's nothing new to re-review on the code side this round. On the non-blocking Low item (a deterministic test that forces the `CooperativeTaskWorker` race rather than relying on timing): agreed this is a nice-to-have, not a blocker — I'm fine leaving it out of this PR's scope unless a maintainer specifically wants it landed here. As I have comment-only review rights on this repo, a maintainer with write-level review permission still needs to give the formal approve/merge — but from a correctness and safety standpoint, I don't see anything left blocking this PR. -- 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]
