SEZ9 commented on PR #11408: URL: https://github.com/apache/seatunnel/pull/11408#issuecomment-5390957914
Thanks @DanielLeens — confirmed on my side as well, and yes, the sequencing you describe matches what I see: the head is still `f0b0eb1e1d36` with no new commit since your July 11 review and the July 14 / August 6 rechecks, so there is nothing new for either of us to re-verify yet. Agreed on all four blockers as summarized: 1. The dead-code guard — `lastKnownGoodTs` is always assigned equal to `resolvedTs`, so the reset path can never fire. 2. The duplicated/ambiguous constructors that default to `None`. 3. The `None`-arithmetic crash risk that follows from those defaults. 4. Most fundamentally, none of `solution_0.py`–`solution_5.py` live inside `seatunnel-connectors-v2/connector-tidb-cdc` or anywhere else in the Maven build, so no Zeta/Flink/Spark job executes any of this code. Changes-requested stands from my side too. To make the remaining asks concrete for the next push: - Land a real Java-side commit against the TiDB-CDC connector implementing the resolved-ts fix, and remove the `solution_*.py` files entirely. - Resolve the constructor/default ambiguity so there is a single, well-defined initialization path (no `None` defaults feeding arithmetic). - Add regression coverage for the resolved-ts reset behavior and ASF headers on any new files. - Sync with the latest `dev` (the branch is currently behind) and rerun the failing Build gate (https://github.com/apache/seatunnel/runs/86563995334) so we can separate baseline CI noise from the code-side fixes. Once a new head lands with the above, I'm happy to do the full re-review alongside you as proposed. <!-- streview-comment:503 --> -- 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]
