fenil25 commented on PR #14395: URL: https://github.com/apache/iceberg/pull/14395#issuecomment-3487485986
> @fenil25 Thank you so much for this thoughtful change — really appreciate your effort! Just wanted to share that this was already discussed with @bryanck a while back, and we’d agreed that the check in CommitterImpl is indeed redundant. I was about to make this change but was a little busy and a bunch of resiliency tests were pending to cover some scenarios. The client can’t guarantee server-side invariants, so removing it makes perfect sense. Also if you see the consumer coordinator class of kafka, it does check stability before proceeding with the open and close calls. Your change aligns perfectly with that direction. Just before going ahead with this, it would be nice if we can add some unit and integration tests on this one. Perfect. Thanks a lot for the insights! Really appreciate your eyes. I added some tests 🙇♂️ -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
