SEZ9 commented on PR #12366: URL: https://github.com/apache/seatunnel/pull/12366#issuecomment-5903273529
@201811510411lw Thanks for the update and for syncing with `dev` after #12449 landed. I'm fine with keeping this PR scoped to the routing correctness fix. The remaining items from the review (skipping per-row work when routing isn't needed, defining behavior for rows whose table isn't in the wrapper, and a container-based E2E for the fixed-bucket + parallel writer scenario) are improvements on top of the fix rather than blockers, so follow-up PRs are acceptable. A few things before this goes in: 1. Since CI on `709750af9` only passed on the second attempt ([run 36367411220](https://github.com/201811510411lw/seatunnel/actions/runs/36367411220)), could you note which tests failed on the first attempt? If it was `SplitClusterFaultToleranceIT` or `BackpressureSlowSinkIT` again, that's consistent with #12353/#12311 and #12313/#12316 and I'm not worried. If anything else failed, I'd like to see it before merging. 2. Please open tracking issues (or at least a short checklist in the PR description) for the three follow-ups above so they don't get lost once this is merged. A link to them in the description is enough. 3. The `java.util.Optional` FQN in `SinkDataPartitioner.java` compiles fine, as you say, but since it's a one-line change I'd prefer to just take the import now rather than carry it into a follow-up. 4. Please double-check that the `incompatible-changes.md` entries (en/zh) still read correctly after the merge with `dev`, since unrelated entries landed around yours. Once 1–3 are addressed I'll do a final pass and move to merge. <!-- streview-comment:1424 --> -- 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]
