DanielLeens commented on PR #12129:
URL: https://github.com/apache/seatunnel/pull/12129#issuecomment-5775034184

   Thanks for syncing with `dev` (`8d66e18f`), @akshar27, and thanks for 
pressing for a fresh confirmation against the new head, @SEZ9 — fair ask, since 
a merge commit is exactly the kind of thing that deserves a quick verification 
rather than being taken on trust.
   
   **Confirming the six points are unchanged from `408f1e0`:** I diffed 
`PhysicalPlan.java` and `JobStatusTest.java` directly between `408f1e0` and 
`8d66e18f` and it is empty — the sync only pulled in unrelated `dev` commits 
(including #12287's minio fix), it did not touch either file. So everything 
verified in my APPROVED review against `408f1e0` — the 
`Collections.unmodifiableSet` wrap, the `SCHEDULED`-exclusion Javadoc, the 
null-fail-fast behavior in `cancelJob()`/`stopJob()`, and the pinning test's 
count-plus-per-index assertions with the same-build-only Javadoc scope note — 
still holds byte-for-byte on `8d66e18f`. Nothing to re-review there.
   
   **CI on `8d66e18f`:** a real run happened this time. Good news first — 
`all-connectors-it-6` and the `unit-test` jobs (which run `JobStatusTest`) are 
green on both JDK 8 and 11, and #12287's minio fix did clear the earlier 
`IcebergSourceIT` 404. Two jobs are still red, but neither is caused by this 
PR's diff:
   
   - `all-connectors-it-2` (JDK 8 and 11): fails in 
`connector-cdc-opengauss-e2e`, a module this PR does not touch at all.
   - `engine-v2-it` — only the JDK 11 shard fails (JDK 8 is green), in 
`CheckpointCoordinatorFailoverIT.testStreamJobFailsAfterCheckpointTriggerDispatchFailure`.
 That test was only just added to `dev` by an unrelated PR (#12199) and landed 
on this branch purely through the sync; this PR never touches 
`CheckpointCoordinator` or anything in that test's path. The 
JDK-8-passes/JDK-11-fails split is also consistent with test flakiness rather 
than a real regression, since nothing in this PR's logic is 
JDK-version-dependent.
   
   So from my side: a rerun of just those two jobs is worth trying, but if they 
stay red it is not something to chase in this PR's own code — flag it 
separately if it keeps failing after a rerun. Everything this PR actually 
touches (`PhysicalPlan`, `JobStatusTest`) is green. My APPROVED review stands.
   


-- 
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]

Reply via email to