SEZ9 commented on PR #11486: URL: https://github.com/apache/seatunnel/pull/11486#issuecomment-5421601986
Thanks for the crisp summary, @DanielLeens — it matches my read exactly, and to answer you directly: yes, I'm aligned on all points, and nothing has landed yet, so both blocking items are still outstanding. To restate the merge criteria so there's no ambiguity: 1. **Issue 2 (blocking):** a test proving the final visible records are still delivered — not trimmed away — when a split finishes in the same fetch. Agreed this is the core behavior the PR fixes, and the current empty-poll-only coverage doesn't exercise it. 2. **Issue 3 follow-through (blocking, trivial):** the one-line `stoppingOffsets` retention cleanup alongside the existing `unassignPartitions` call. Agreed the original Issue 3 concern is withdrawn and superseded by this cleanup, so bundling it in the same push as the test makes sense. Also confirming the rest of the disposition: Issue 1 (bounded/guarded `position()` call) is a defensive-hardening suggestion rather than a blocker; Issue 5 is cosmetic since `currentOffset` only feeds a `LOG.debug` line; and Issues 4 (closing the consumer leaked by the constructor in the reflection-injected test), 6 (docs note on bounded reads with transactional topics), and 7 (test Javadoc) remain non-blocking follow-ups — happy to see any of them in this PR if convenient, but none gate approval. Once a push lands with the final-records test and the `stoppingOffsets` removal, I'll take a final pass and approve. Nothing else needed from my side. <!-- streview-comment:562 --> -- 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]
