Thanks for the feedback Edo.

1) While the initial name was matching the Protocol, I agree it was
not great for users. I've updated commitOffsets() to
commitConsumerGroupOffsets() as suggested.

2) I thought passing the timestamp for each partition would make it
harder to use in the most common case (retrieving offsets for the same
timestamp to several partitions). That said, the offsetForTimes() API
of Consumer requires that already, so I've updated the KIP fix it.
listOffsets() now takes a Map of TopicPartition to Long and the
isolation level is still in the Option object.

On Fri, Nov 30, 2018 at 11:57 AM Edoardo Comar <eco...@uk.ibm.com> wrote:
>
> Thanks for the KIP Mickael!
>
> two initial observations
>
> `commitOffsets` methods could be renamed to `commitConsumerGroupOffsets`
> to better match the existing `listConsumerGroupOffsets`
>
> `ListOffsetsOptions` offers you a per-request isolation level option and
> timestamp, but timestamp is a per-partition field in the Kafka ListOffsets
> Request API
> so you're not exposing the full APi power with the option
>
>
> --------------------------------------------------
>
> Edoardo Comar
>
> IBM Event Streams
> IBM UK Ltd, Hursley Park, SO21 2JN
>
>
>
>
> From:   Mickael Maison <mickael.mai...@gmail.com>
> To:     dev <dev@kafka.apache.org>
> Date:   30/11/2018 10:51
> Subject:        [DISCUSS] KIP-396: Add Commit/List Offsets Operations to
> AdminClient
>
>
>
> Hi,
>
> I've written a KIP to add commit/list offsets operation to the
> AdminClient:
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97551484
>
>
> Please take a look and let me know if you have any feedback. Thanks
> Mickael
>
>
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Reply via email to