Hey Guozhang, thanks a lot for proposing the KIP so fast! Two high level comments are:
1. In the code we define the function to extract admin client id as `getShardAdminClientId`. Do we also want to add `shared` keyword in the public API so that people know the admin client is not per-thread basis? 2. KIP-414 is also occupied by Ryanne for https://cwiki.apache.org/confluence/display/KAFKA/KIP-414%3A+Notify+SourceTask+of+ACK%27d+offsets%2C+metadata, maybe you want to resolve this conflict with him? Best, Boyang ________________________________ From: Matthias J. Sax <matth...@confluent.io> Sent: Sunday, January 13, 2019 10:30 AM To: dev@kafka.apache.org Subject: Re: [VOTE] KIP-414: Expose Embedded ClientIds in Kafka Streams Thanks for the KIP. One side comment about rejected alternatives. I would remove the sentence, because `StreamsMetadata` is part of IQ feature and not related at all to this change -- thus, it does not seem to be a valid alternative. +1 (binding) -Matthias On 1/11/19 5:47 AM, Bill Bejeck wrote: > Thanks for the KIP Guozhang, > > +1 for me. > > On Fri, Jan 11, 2019 at 7:10 AM Damian Guy <damian....@gmail.com> wrote: > >> +1 >> >> On Fri, 11 Jan 2019 at 05:09, John Roesler <j...@confluent.io> wrote: >> >>> Hi Guozhang, >>> >>> It sounds reasonable to me. I'm +1 (nonbinding). >>> >>> -John >>> >>> On Tue, Jan 8, 2019 at 8:51 PM Guozhang Wang <wangg...@gmail.com> wrote: >>> >>>> Hello folks, >>>> >>>> I'd like to start a voting process for the following KIP: >>>> >>>> >>>> >>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-414%3A+Expose+Embedded+ClientIds+in+Kafka+Streams >>>> >>>> It is a pretty straight-forward and small augment to Stream's public >>>> ThreadMetadata interface, as an outcome of the discussion on KIP-345. >>> Hence >>>> I think we can skip the DISCUSS thread and move on to voting directly. >> If >>>> people have any questions about the context of KIP-414 or KIP-345, >> please >>>> feel free to read these two wiki pages and let me know. >>>> >>>> >>>> -- Guozhang >>>> >>> >> >
