DanielLeens commented on PR #11271: URL: https://github.com/apache/seatunnel/pull/11271#issuecomment-5409164774
Thanks for the quick turnaround, @hutiefang76 — and nicely done tracking that down. I checked the current head (`96141c4267`) directly: it does include `43fe63b1fc` (#11954, the `FAILED_JOB_EVENT_TIMEOUT_SECONDS` fix) in its history, so the rebase landed exactly where I suggested in my last round. My approval from earlier today was already on this exact commit, so nothing changes there on my end — the source-level review still stands (byte-identical CDC logic to what I traced line-by-line, docs entries closing Issue 4). On the local test run: `IncrementalSourceReaderTest`, `IncrementalSplitTest`, and `IncrementalSplitAssignerTest` are exactly the three test classes covering the changed `connector-cdc-base` logic, so 11/11 passing locally lines up with what I'd expect given the source is unchanged from the version I already approved. One thing worth flagging while it's fresh: I looked at the fork's `Build` run for this head (`hutiefang76/seatunnel` run `32745169272`) and it's still landing on `cancelled` rather than a clean `success`, across a couple of attempts. Everything that did complete came back green — including `unit-test (8, ubuntu-latest)`, which is where these three CDC test classes would run — but `mysql-cdc-connector-it`, `paimon-connector-it`, `elasticsearch-connector-it`, `unit-test (11, ubuntu-latest)`, `unit-test (11, windows-latest)`, and `Build website` all got cut off mid-run rather than finishing (attempt 2 cancelled `mysql-cdc-connector-it`/`doris-connector-it`/`paimon-connector-it (11)`, attempt 3's re-run of the survivors cancelled again). Nothing in what I can see points to an actual test failure — it looks like the run is getting cut off rather than red — but it's worth one more clean rerun (or splitting the retry across a bit more time) so the `Build` check actually reflects a finished state before merge, since a `cancelled` conclusion won't satisfy the required check either way. To be clear on where things stand: my side has no outstanding blockers on the code itself. The one open item on the PR is @SEZ9's CHANGES_REQUESTED review (Issue 1 there was resolved back in `e381b8eb`, and the remaining items 2/3/5/6/7/8 in my summary table are non-blocking, in my view) — I don't have dismiss rights on someone else's review, so that one will need their own follow-up or a maintainer call. Really appreciate you sticking with this through this many rounds. -- 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]
