SEZ9 commented on PR #11678: URL: https://github.com/apache/seatunnel/pull/11678#issuecomment-5358312506
Thanks @DanielLeens for the thorough round-5 and round-6 write-ups — the level of detail here (verifying `9f038035b` touches only `JdbcSinkWriter.java` and that `ConnectionPoolManager.java`, `SimpleJdbcConnectionPoolProviderProxy.java`, and `ConnectionPoolManagerTest.java` are byte-identical to the round-4 approved version) makes this easy to follow. On the account mix-up: understood, no problem. I'll treat `#pullrequestreview-4957017931` as the authoritative round-5 record and will dismiss the stray duplicate at `#pullrequestreview-4957008079` so the review history stays clean. Good catch on the interaction with the Error Data Bypass / Dead Letter Queue feature that landed in `dev` after your round-4 approval — that's exactly the kind of thing a full re-review of the head (rather than an incremental diff) is meant to surface, and I appreciate you tracing it rather than assuming the merge commit `8add67a` was content-neutral. Two concrete asks before we move to merge: 1. Your round-6 comment says `9f038035b` fixes both Medium-severity blockers from round 5, but the visible analysis covers Fix 1 (`rollbackIfNeeded()`). Could you confirm explicitly that the second blocker is also verified resolved on the current head, ideally with the same read-the-method-body verification you did for Fix 1? 2. Please confirm there's nothing else outstanding from your side on `bb6d4dc` (the cherry-picked flake fix) — my understanding from your notes is it's already-merged content and adds no new PR-authored logic, but I'd like that stated for the record. Once both are confirmed, I'm comfortable proceeding with the merge given your APPROVED review. Thanks again for the careful work here. <!-- streview-comment:410 --> -- 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]
