DanielLeens commented on PR #11512: URL: https://github.com/apache/seatunnel/pull/11512#issuecomment-6313677587
Thanks @SEZ9 for going through the F1-F8 table, and @goutamadwant for your patience. A quick follow-up on the two points in the last comment, checked against head `d2f8193c97` (source read only, nothing run locally; I do not see a newer commit since my last review on this head). **Local test run:** the required `Build` check is green on this exact head, so the referenced tests (`CdcProgressModelTest`, `ReportCdcProgressOperationSerializationTest`) run in the project CI rather than only on a contributor machine. A separate local run by the author is optional from my side. **F7 Javadoc note:** I agree with the suggestion, and it is still outstanding on this head. The checked cross-validation lives in `validateExactSplitCounts` (it only enforces `assigned == completed + running` when all three counts are `EXACT`), but neither the class Javadoc nor the constructor at `CdcEnumeratorProgressReport.java:89-132` mentions that condition, so a connector author passing `ESTIMATED` or `UNKNOWN` counts has no hint why they are not cross-checked. @goutamadwant, a two-line constructor Javadoc (counts must be non-negative; the `assigned == completed + running` invariant is enforced only when all three are `EXACT`) would close it. It can go in the same push as the `dev` sync below. **Still pending, unchanged:** `mergeStateStatus` is `DIRTY` (conflicts with `dev`), so the branch needs a sync before the final pass. My non-blocking carryover on string-length and frame-size bounds in `CdcProgressReportSerializer` is also unchanged. Nothing else is open from my side. Thanks again! -- 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]
