SEZ9 commented on PR #12156: URL: https://github.com/apache/seatunnel/pull/12156#issuecomment-5902926378
Thanks @CryoThrust for the update on the PR description. On F2, the plan to defer the check-then-act window between the not-started check and `updateJobState(CANCELED)` in `cancelJob()`/`stopJob()` to a follow-up issue is fine with me, as long as the link stays discoverable after merge. I'll treat F2 as pending until I've confirmed the link is present in the description myself. F1, F3 and F4 I still need to verify against the diff at head `449b517823f5dd4d231634eea32a52d8e5f191dc` before signing off; I'll do that and update here. Nothing new to add to the review scope. On CI: I can't classify the failing partitions from this thread alone. Since this PR touches the Zeta job-status logic, `unit-test (8, ubuntu-latest)` and `engine-v2-it (8, ubuntu-latest)` are the two I'd want to rule out before calling the failures unrelated. Could you share the failing test names / stack traces (or a link to the job logs) for those two jobs, and whether the same jobs fail on the current dev baseline without this PR? If they turn out to be unrelated to `PhysicalPlan`/`JobStatus`, I'm happy to rerun and proceed. <!-- streview-comment:1416 --> -- 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]
