SEZ9 commented on PR #11408: URL: https://github.com/apache/seatunnel/pull/11408#issuecomment-5381236950
@DanielLeens — confirming your sequencing question directly: yes, the head is still `f0b0eb1e1d36`, the only commit on the branch, and nothing new has landed since your July 11 review and July 14 recheck. Reconciling our reviews rather than doing a fresh pass was the right call, and I agree with your reconciliation on all points. To recap where we've converged: - **Issue 2 (dead code):** Agreed. Since `lastKnownGoodTs` is assigned in the same block that advances `resolvedTs` in both `solution_0.py` and `solution_1.py`, the difference is always `0`, the `> 1000` branch in `checkResolvedTs` is unreachable, and the advertised "reset to last known good" behavior never executes. - **Issue 4 (None-default crash):** Agreed, and thanks for the extra evidence — the duplicate class declarations in `solution_4.py`/`solution_5.py` pulling `None` defaults from the configs in `solution_2.py`/`solution_3.py` make it ambiguous which constructor is even intended, on top of the `None`-arithmetic crash. - **Issue 3 (not wired into the build):** Agreed this matches your original Issue 1 — none of `solution_0.py`–`solution_5.py` sit inside `seatunnel-connectors-v2/connector-tidb-cdc` or the Maven build, so no Zeta, Flink, or Spark job executes any of this. This remains the fundamental blocker regardless of the internal logic bugs. So the changes-requested state stands unchanged. Concrete asks before a fresh pass makes sense: 1. Remove all `solution_*.py` files from the PR. 2. Implement the actual fix in Java within the TiDB CDC connector's resolved-ts path, resolving the constructor/default ambiguity from Issue 4 rather than carrying both variants over. 3. Add regression coverage and ASF license headers on any new files. 4. Sync the branch with the latest `dev` (it was `ahead_by=1`, `behind_by=9` at last check) and rerun the failing `FAILURE gate: Build` (run `86563995334`) so we can separate baseline CI noise from code-side issues. Once a real Java-side commit is pushed, I'm happy to join a full re-review alongside you. Thanks again for the careful independent verification — it made reconciling the two reviews straightforward. <!-- streview-comment:448 --> -- 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]
