SEZ9 commented on PR #11512: URL: https://github.com/apache/seatunnel/pull/11512#issuecomment-5612054356
@DanielLeens thanks for doing a full fresh re-review of `b921264bb` rather than treating it as a test-only bump, and for the note on the F4 paraphrase. Noted that the doc text itself was unchanged and describes a coordinator-derived request/poll model, which matches what @goutamadwant stated in the correction. Since the only diffed change in `b921264bb` is the test file, I'd still like F4 grounded in the actual coordinator collection code before closing it — could you point at where the collection path is implemented so we can confirm it matches the doc? On the two clarifications that led to this commit: - **F8**: the new `CdcProgressModelTest.testEnumeratorReportActiveSplitWatermarksAreDeeplyImmutable` covers mutation through the original inputs and through the returned collections after construction. @DanielLeens, you mentioned re-reading `CdcProgressPosition.java`, `CdcSnapshotSplitProgress.java` and `CdcEnumeratorProgressReport.java` to confirm the guarantee — could you state your conclusion on that so F8 has an explicit record? - **F7**: @goutamadwant pointed at the existing count-validation coverage. @goutamadwant, since the ask was specifically a test-name pointer, could you post the exact method name(s) in `CdcProgressModelTest` that exercise the negative/inconsistent count rejection? @DanielLeens, please also confirm whether, against `b921264bb`, all of F1–F8 are resolved from your side, or list any you still consider open. @goutamadwant, you mentioned new-head CI was still pending — please ping here once it has finished on `b921264bb` and I'll take it from there. <!-- streview-comment:944 --> -- 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]
