Thanks Matthias, 1) Updated the KIP page to include KAFKA-6126. 2) For passing configs, I agree, will make a pass over the existing configs passed to StreamsKafkaClient and update the wiki page accordingly, to capture all changes that would happen for the replacement in this single KIP. 3) For internal topic purging, I'm not sure if we need to include this as a public change since internal topics are meant for abstracted away from the Streams users; they should not leverage such internal topics elsewhere themselves. The only thing I can think of is for Kafka operators this would mean that such internal topics would be largely reduced in their footprint, but that would not be needed in the KIP as well.
Guozhang On Sat, Nov 4, 2017 at 9:00 AM, Matthias J. Sax <matth...@confluent.io> wrote: > I like this KIP. Can you also link to > https://issues.apache.org/jira/browse/KAFKA-6126 in the KIP? > > What I am wondering though: if we start to partially (ie, step by step) > replace the existing StreamsKafkaClient with Java AdminClient, don't we > need more KIPs? For example, if we use purge-api for internal topics, it > seems like a change that requires a KIP. Similar for passing configs -- > the old client might have different config than the old client? Can we > double check this? > > Thus, it might make sense to replace the old client with the new one in > one shot. > > > -Matthias > > On 11/4/17 4:01 AM, Ted Yu wrote: > > Looks good overall. > > > > bq. the creation within StreamsPartitionAssignor > > > > Typo above: should be StreamPartitionAssignor > > > > On Fri, Nov 3, 2017 at 4:49 PM, Guozhang Wang <wangg...@gmail.com> > wrote: > > > >> Hello folks, > >> > >> I have filed a new KIP on adding AdminClient into Streams for internal > >> topic management. > >> > >> Looking for feedback on > >> > >> *https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >> 220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier > >> <https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >> 220%3A+Add+AdminClient+into+Kafka+Streams%27+ClientSupplier>* > >> > >> -- > >> -- Guozhang > >> > > > > -- -- Guozhang
