SEZ9 commented on PR #11015: URL: https://github.com/apache/seatunnel/pull/11015#issuecomment-5077822229
Thanks @DanielLeens for the detailed recheck — that clears up both of my compatibility warnings. - `MultiTableSinkWriterTest.java` being test code means the signature change there isn't a downstream-facing API, so I'm dropping Issue 1. - Keeping the original two-argument `CustomMySqlAntlrDdlParser` constructor as a deprecated overload that delegates to the new one addresses the source/binary compatibility concern in Issue 2, so that's resolved from my side as well. That said, this doesn't unblock the PR yet. Remaining asks before merge: 1. **Fix the barrier-enqueue/worker-exit lost-notification window** flagged in Daniel's self-review — that's now the real blocker on this head. Please push a fix (or explain why the window can't be hit in practice) so it can be re-verified. 2. **Sync with the latest `dev`** — the branch has diverged (ahead 9 / behind 9), and the current `Build` gate is failing: https://github.com/apache/seatunnel/runs/86601189863. Please rebase/merge and rerun CI. If it still fails on the updated head, share the log and we can help narrow it down. Once the lost-notification fix lands and CI is green on a synced head, I'll do the final review pass and handle the approval/merge step. Thanks both for the thorough work here. <!-- streview-comment:1 --> -- 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]
