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
>

Reply via email to