DanielLeens commented on PR #11408: URL: https://github.com/apache/seatunnel/pull/11408#issuecomment-5385672846
Thanks @SEZ9 — confirmed, agreed on all points, and the sequencing is right: this is still head `f0b0eb1e1d36`, the same one I reviewed on July 11 and rechecked on July 14 and August 6, with no new commit since. Good to have your independent re-verification converge on the same four blockers: the dead-code guard (`lastKnownGoodTs` always assigned equal to `resolvedTs`), the duplicated/ambiguous constructors that default to `None`, the `None`-arithmetic crash risk, and — most fundamentally — the fact that none of `solution_0.py`-`solution_5.py` sit inside `seatunnel-connectors-v2/connector-tidb-cdc` or anywhere else in the Maven build, so no Zeta/Flink/Spark job executes any of this. Two independent traces landing on the same root causes gives strong confidence this is the real state rather than an artifact of either of our individual reads. Changes-requested stands, unchanged. Once a real Java-side commit lands against the TiDB-CDC connector implementing the resolved-ts fix (removing the `solution_*.py` files, resolving the constructor/default ambiguity, and adding regression coverage + ASF headers on any new files), I'm glad to join a full re-review alongside you on the new head. -- 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]
