Thank you David, I retitled this KIP to be more accurate and supplemented the Compatibility and Rejected Alternatives sections, please help to review this again.
Best, Ziming Deng > On Jan 31, 2022, at 6:12 PM, David Jacot <[email protected]> wrote: > > Hey, > > Thanks for the KIP. I have a few comments: > > 1. I think that it would be better to name the KIP: "GetOffsetShell > should support max-timestamp" > or something like that as this is the initial intent of the change. > > 2. There is a typo: `OffsetSpce` -> `OffsetSpec`. > > 3. It would be great if we could further expand the compatibility > section. It seems that the number > of consumer configurations which could reasonably be used by > `GetOffsetShell` is quite small (timeout, > retries, etc.) and it seems that most of them (if not all) are > supported by the admin client as well. I wonder > if we could be explicit here and argue that the transition won't be > noticed. I might be speculating here. > > 4. For completeness, I think that we should mention extending the > consumer to support max-timestamp > as well in the rejected alternatives. That would be another way to > address the issue. However, I agree > with you that using the admin client is better in the admin tools. > > Best, > David > > On Sun, Jan 30, 2022 at 2:09 PM deng ziming <[email protected]> wrote: >> >> Sorry all, I mean KIP-851 not KIP-734. >> In KIP-734 we add a new OffsetSpec to AdminClient, in this KIP I just extend >> this OffsetSpec to GetOffsetShell. >> >>> On Jan 30, 2022, at 6:29 PM, deng ziming <[email protected]> wrote: >>> >>> Hey all, I'm starting the voting on KIP-734. >>> >>> This supports a new OffsetSpec in GetOffsetShell so that we can easily >>> determine the offset and timestamp of the message with the largest >>> timestamp on a partition. This seems a simple change but replaced >>> KafkaConsumer with AdminClient in GetOffsetShell. >>> >>> Thanks, >>> Ziming Deng >>
