SEZ9 commented on PR #11512: URL: https://github.com/apache/seatunnel/pull/11512#issuecomment-5642914886
Thanks for pushing the new head (`5f04cb68f4`). I have not re-audited the diff; this is a status pass on the eight findings from my review at `b921264bb490`, and I'd like to close each one out explicitly rather than infer from the automated summary above. A couple of notes prompted by that summary, all inside the existing findings: - **F1 (bounded `activeSplits`)** — the summary still describes the enumerator report as carrying "bounded active-split detail". The finding was that `CdcEnumeratorProgressReport.activeSplits` accepts any list size with no cap. If a cap is now enforced (constructor/factory truncation or rejection), please point me to it; if the bound is only documented, that's still open. - **F4 (pull vs. push docs)** — the traced flow shows the worker pushing reports to the master via `ReportCdcProgressOperation` on a scheduled executor, while the "After" wording talks about `CoordinatorService` pulling "on demand". The developer doc needs to describe the path that actually exists (worker-side scheduled push, coordinator retains latest per source task, exposure reads the retained copy). Please confirm the doc text now matches. - **F8 (immutability)** — "immutable progress-report types" is asserted again; the finding was that the report only shallow-copies the list, so immutability depends on `CdcSnapshotSplitProgress` being immutable itself. Either make that guarantee explicit in the type or soften the Javadoc claim. For the remaining ones I have nothing new to add and just need a yes/no plus pointer: - **F2** — does the payload contract in `docs/en/developer/cdc-progress.md` now forbid credentials/connection secrets in connector-native position payloads? - **F3** — does the engine codec encode `CdcProgressAccuracy` (and the other public enums in the report) by name rather than ordinal? - **F5** — does the doc list which connectors currently implement the provider? - **F6** — has the `CdcProgressLifecycle.SNAPSHOT` Javadoc been narrowed so it no longer includes enumerator-owned discovery/assignment? - **F7** — are negative / mutually inconsistent counts now rejected (or at least documented as caller-validated)? Also, the automated comment above references an "Issue 1" but the body is cut off before it. Could you paste what it flagged so I can tell whether it overlaps one of F1–F8 or is something you've already handled? A short per-finding reply (fixed in this head / deferred with reason) is all I need to move this forward. <!-- streview-comment:980 --> -- 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]
