SEZ9 commented on PR #11721: URL: https://github.com/apache/seatunnel/pull/11721#issuecomment-5578435823
Thanks both for the thorough follow-up here. @SEPURI-SAI-KRISHNA — appreciate the clear explanation of the `Build` check lag. @DanielLeens's independent re-check that `aea9854a1bb1` now reports `Build: success` (run `97507796273`) after the `kudu-connector-it (11, ubuntu-latest)` rerun matches what you described, and since this diff doesn't touch anything under `connector-kudu`, I'm treating the earlier `cancelled` state as unrelated noise. On the two points from the previous review: - **PR11721-F1 (docs snippet vs. real implementation in `docs/en/architecture/features/multi-table.md`)** — you mentioned the branch now retires the known-issue callout that this fix makes obsolete. Could you confirm that the code snippet shown in that doc now mirrors the actual `MultiTableSinkWriter` routing code (i.e. the `replicaNum` / `extractPrimaryKeyIfPresent` naming vs. `blockingQueues.size()` / `element.getField` mismatch is gone), rather than just the callout being removed? A one-line pointer to the updated section is enough. - **PR11721-F2 (shared non-negative-mod helper in `MultiTableSinkWriter.java`)** — @DanielLeens notes this is closed out with a follow-up tracked separately. Just so the record on this thread is unambiguous: is the helper extraction landing in this PR, or is this PR keeping the inline `(hash & Integer.MAX_VALUE) % n` idiom with the extraction deferred to that follow-up? Either is fine given the new test pinning the exact target queue for the `Integer.MIN_VALUE` case, I'd just like the intent stated explicitly here. Once those two confirmations are in, I have nothing further on my side and will move this along for the formal approval needed to clear branch protection. <!-- streview-comment:898 --> -- 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]
