DanielLeens commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5728628355
@SEZ9 — I independently re-checked the current head (`2dc21468105`) before replying, rather than just taking @abdessalems's word for it: - The production diff in `TaskExecutionService.java` really is exactly 3 hunks: the import removal (`@@ -73,7 +73,6 @@`) and the two hunks inside `BlockingWorker`'s constructor/`run()` (`@@ -1277...@@`, `@@ -1308...@@`). Neither touches `deployLocalTask()`'s body — the only occurrences of `deployLocalTask` anywhere in the full diff are new call sites in `TaskDeployStaleContextRaceTest.java` and `TaskExecutionServiceTest.java`, which exercise the existing method rather than change it. - `finishOwnedResources`, `finishExecutionContext`, and `cancelOwnedAsyncFunctionsInPlace` all come back with 0 hits against the current diff, matching your grep. That ownership model is genuinely gone — it lived in the three commits cherry-picked from #11757 (closed unmerged), and #12238 replaced it with a different, identity-checked `compute()` design. So on the ambiguity you raised: @abdessalems's "not touched by this diff" does mean "no longer part of this PR at all," not "unchanged since the last push." F4/F6/F8 targeted code that simply isn't in this PR anymore — if you still think they apply, that would need to be raised against #12238 itself, not here. F1/F3/F5 are a genuinely separate question: they're about `dev`'s pre-existing `deployLocalTask()` locking/`put` semantics, which this PR doesn't touch in either direction. I'd treat those as valid `dev`-level concerns to track against #12238 rather than blockers on this specific 3-hunk PR, but I understand wanting more than an argument to rely on — a short comment in the test pointing at exactly where that race lives (as offered for F7 coverage) seems like a reasonable, low-cost breadcrumb either way. My source-level review of this PR's actual diff stands as I left it. Happy to take another pass once you've confirmed F2 against the diff yourself. -- 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]
