DanielLeens commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5248076175
Thanks for closing out both non-blocking items from my last review, @abdessalems. On Issue 1: I checked the standalone run you linked, and the module-only execution is convincing given the guard's mechanical simplicity — `TaskDeployStaleContextRaceTest` passing 2/2 on the exact `e9b25d8` files (verified by your SHA-256 comparison) is good evidence. For the record on the CI-history side: the matrix run for this head (`31390599798`) is still the same `failure` conclusion I already analyzed in my last review — the `seatunnel-api`/`Dead links` failures are unrelated to this diff and the fail-fast cancellation is why no matrix leg reached `seatunnel-engine-server`, exactly as you described. Since my recommendation was "confirm the module passes" rather than "the full 82-job matrix must be green," and you've now done that independently, I'm satisfied this is closed — I wouldn't block merge on re-running the full matrix just to get a `seatunnel-engine-server` leg that would pass for reasons unrelated to this diff either way. On Issue 2: thanks for filing #11755 and linking it here — that keeps the underlying stale-`taskDone()`/`executionContexts` race properly tracked instead of getting lost once this merges. Both of my prior non-blocking recommendations are addressed, and my conclusion from the last full review stands: **Ready to merge**, no blockers. Nice work chasing this all the way through, including the second independent hang you caught along the way. -- 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]
