DanielLeens commented on PR #12156: URL: https://github.com/apache/seatunnel/pull/12156#issuecomment-6210070584
Thanks for the checklist, @SEZ9. This crossed paths with my review of the same head (`0cd5d8544e4`, review 5516481221), so here is the same confirmation in one place, checked against the code at that head rather than the PR description: - **F1**: confirmed. `cancelJob()` captures `JobStatus jobStatus = getJobStatus();` at `PhysicalPlan.java:216`, guards on `jobStatus.isEndState()`, and then branches with `if (NOT_STARTED_STATUSES.contains(jobStatus))` at `:224`. `stopJob()` does the same at `:263` and `:269`. The second `runningJobStateIMap.get(jobId)` read in `cancelJob()` is gone. - **F3**: both halves are covered. The enum-order guard `JobStatusTest#testOrdinalOrderIsStableForInternalRpcCompatibility` has a Javadoc at `JobStatusTest.java:42` telling a future contributor to append new statuses and not to refresh the expected list. For the `PhysicalPlan` decision, `StateTransitionCleanupTest` has `testCancelJobCommitsToTheJobStatusSnapshotItValidated` (`:174`), `testCancelOnNotStartedJobGoesStraightToCanceled` (`:200`) and `testCancelOnRunningJobGoesThroughCanceling` (`:213`). The first one makes the first IMap read return `PENDING` and a later read return `RUNNING`, so it fails if `cancelJob()` goes back to a second read. - **F4**: the Javadoc on `NOT_STARTED_STATUSES` (`:54-65`) carries the explanation and the #12342 pointer, and the two call-site comments that duplicated the set contents are removed in the `dev...head` diff. - **F2**: agreed that it stays deferred to #12342. So from the source side there is nothing left on this head. The only open gate is CI: the apache-side `Build` check is still `action_required` and the fork workflows have to be re-enabled by @CryoThrust before a green run can be linked. A committer approval is still needed as well, since my account is comment-only. -- 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]
