Hi Boyang, Thanks for the feedback!
1. I did not want to expose this information since we want to have the flexibility to change its internal impls while exposing the clientIds: e.g. if in the future we've decided to let each thread use their own admin client, we'd not need to change the function name. 2. Thanks for the reminder! I saw Matthias has helped on resolving it and left a comment on the other KIP. Guozhang On Sat, Jan 12, 2019 at 8:53 PM Boyang Chen <bche...@outlook.com> wrote: > 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 > >>>> > >>> > >> > > > > -- -- Guozhang
