abdessalems commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5617534909
@SEZ9 — pushed f1e02f942 with the F3 rewording and the F8 comment. On F3: I think the original text was accurate, but clearly not clear enough. deployTask(TaskGroupImmutableInformation) opens synchronized (this) at line 528 and the deployLocalTask(...) call at line 550 sits inside that block, so the redeploy guard and the deploy really do run under the same monitor as this teardown — the claim was about deployTask, not deployLocalTask. I've reworded it to name the enclosing block explicitly, say plainly that a caller reaching deployLocalTask directly isn't covered, and point at #12164 for that path. On F8: added a comment at finishExecutionContext explaining the stale branch deliberately keeps its context out of finishedExecutionContexts, since the location is reused across generations. On F7: yes, the tests still use reflection — ReflectionUtils.getField for executionContexts, cancellationFutures, taskAsyncFunctionFuture and timerFlushFutures. I didn't add a non-reflective seam; happy to if you'd prefer one, though it would mean widening the production API for tests, so I'd rather you decide that than assume. F1/F5 note added to the PR description, alongside the #12224 reference. Locally both classes are green on this head: 20 tests, 0 failures. -- 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]
