DanielLeens commented on PR #11512: URL: https://github.com/apache/seatunnel/pull/11512#issuecomment-5578515698
@SEZ9 happy to give an explicit record at `95cef1c0850`. Short answer: all 8 are resolved, and none of them have been touched since I last verified each one line-by-line against source (in my 2026-08-31 comment, at head `b1a403bdbb3e`). The two commits between that head and this one (`433e9b3b8bc0`, a routine `dev`-sync merge, and `95cef1c0850`, the test-only strengthening) don't touch any of the files these items live in — I checked both file-by-file in my 2026-09-06 and 2026-09-08 full re-reviews rather than assuming it from the commit messages. - **F1 (bounded `activeSplits`)** — resolved. `CdcEnumeratorProgressReport.java:39` defines `MAX_ACTIVE_SPLITS = 100`; the constructor caps the retained list and sets `activeSplitsTruncated` (lines 65-66, 86, 126-131). Covered by `testEnumeratorReportBoundsActiveSplitDetails`. - **F2 (no-credentials rule for position payloads)** — resolved. `docs/en/developer/cdc-progress.md:38` states position payloads must not "contain credentials, connection URLs, or other authentication material," mirrored in the zh doc and the `CdcProgressPosition` Javadoc. - **F3 (enums encoded by name, not ordinal)** — resolved. `CdcProgressReportSerializer.java` encodes every enum via `.name()` and decodes via `.valueOf(...)` (lines 49, 109, 129, 209, 213, 218, 222, 257); no `.ordinal()` usage in the file. Four exhaustive `testEveryProgress*RoundTripsByName` tests pin this per-constant. - **F4 (runtime collection docs vs. registration-based path)** — resolved. The Runtime collection section of `docs/en/developer/cdc-progress.md` now describes report sources being "registered when coordinator task groups are deployed ... or updates the coordinator-local report directly when the enumerator runs on the master" — matching the actual registration-based implementation, not the old pull/derive wording. - **F5 (connector coverage statement)** — resolved. The "Current limitations" section states: "CDC sources based on `connector-cdc-base` currently provide reports ... CDC sources without this provider wiring return no report." - **F6 (`SNAPSHOT` Javadoc ownership)** — resolved. `CdcProgressLifecycle.java:25` now reads "The reader is reading snapshot splits." — the discovery/assignment wording is gone. - **F7 (count invariant validation)** — resolved. `CdcEnumeratorProgressReport.java:118-123` calls `validateCount(...)` for every count field and `validateExactSplitCounts(...)`, both throwing `IllegalArgumentException` (lines 170-185). Covered by `testEnumeratorReportRejectsInvalidCounts`. - **F8 (element immutability of `CdcSnapshotSplitProgress`)** — resolved. The class is `public final class` with all-`final` fields (lines 33, 36, 39, 42, 45), and its Javadoc now states the deep-immutability guarantee explicitly. Nothing in the 09-06 dev-merge (I traced it file-by-file: it touches `CoordinatorService`, `PeekBlockingQueue`, and their tests, none of which are in this PR's CDC-progress surface) or the 09-07 test-only commit (single file, `CdcProgressServiceTest.java`, purely additive as you already confirmed) reaches any of these 8 items, so my 08-31 verification still stands unchanged at the current head. From my side there is no open source-level item on this PR at `95cef1c0850`. The two things still standing between this and merge are procedural, not code: `Build` is currently `pending` on the fork's run for this exact head (need to see it complete green), and `reviewDecision` is `REVIEW_REQUIRED` with no write-access maintainer approval yet — my own approval is from a comment-only account. Thanks again for the thorough back-and-forth on this one. -- 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]
