Hello Jorge, I made a pass over the wiki, and here are a few comments:
1. First, regarding to Tom's comment #2 above, I think if we are only going to include the String groupId. Then it is Okay to keep as a String than using a new wrapper class. However, I think we could include the protocol_type returned from the ListGroupsResponse along with the groupId. This is a very useful information to tell which consumer groups are from Connect, which ones are from Streams, which ones are user-customized etc. With this, it is reasonable to keep a wrapper class. 2. In ConsumerDescription, could we also add the state, protocol_type (these two are form DescribeGroupResponse), and the Node coordinator (this may be returned from the AdminClient itself) as well? This is also for information consistency with the old client (note that protocol_type was called assignment_strategy there). 3. With 1) / 2) above, maybe we can rename "ConsumerGroupListing" to "ConsumerGroupSummary" and make "ConsumerGroupDescription" an extended class of the former with the additional fields? Guozhang On Tue, Nov 7, 2017 at 2:13 AM, Jorge Esteban Quilcate Otoya < quilcate.jo...@gmail.com> wrote: > Hi Tom, > > 1. You're right. I've updated the KIP accordingly. > 2. Yes, I have add it to keep consistency, but I'd like to know what others > think about this too. > > Cheers, > Jorge. > > El mar., 7 nov. 2017 a las 9:29, Tom Bentley (<t.j.bent...@gmail.com>) > escribió: > > > Hi again Jorge, > > > > A couple of minor points: > > > > 1. ConsumerGroupDescription has the member `name`, but everywhere else > that > > I've seen the term "group id" is used, so perhaps calling it "id" or > > "groupId" would be more consistent. > > 2. I think you've added ConsumerGroupListing for consistency with > > TopicListing. For topics it makes sense because at well as the name there > > is whether the topic is internal. For consumer groups, though there is > just > > the name and having a separate ConsumerGroupListing seems like it doesn't > > add very much, and would mostly get in the way when using the API. I > would > > be interested in what others thought about this. > > > > Cheers, > > > > Tom > > > > On 6 November 2017 at 22:16, Jorge Esteban Quilcate Otoya < > > quilcate.jo...@gmail.com> wrote: > > > > > Thanks for the feedback! > > > > > > @Ted Yu: Links added. > > > > > > KIP updated. Changes: > > > > > > * `#listConsumerGroups(ListConsumerGroupsOptions options)` added to > the > > > API. > > > * `DescribeConsumerGroupResult` and `ConsumerGroupDescription` classes > > > described. > > > > > > Cheers, > > > Jorge. > > > > > > > > > > > > > > > El lun., 6 nov. 2017 a las 20:28, Guozhang Wang (<wangg...@gmail.com>) > > > escribió: > > > > > > > Hi Matthias, > > > > > > > > You meant "list groups" I think? > > > > > > > > Guozhang > > > > > > > > On Mon, Nov 6, 2017 at 11:17 AM, Matthias J. Sax < > > matth...@confluent.io> > > > > wrote: > > > > > > > > > The main goal of this KIP is to enable decoupling StreamsResetter > > from > > > > > core module. For this case (ie, using AdminClient within > > > > > StreamsResetter) we get the group.id from the user as command line > > > > > argument. Thus, I think the KIP is useful without "describe group" > > > > > command to. > > > > > > > > > > I am happy to include "describe group" command in the KIP. Just > want > > to > > > > > point out, that there is no reason to insist on it IMHO. > > > > > > > > > > > > > > > -Matthias > > > > > > > > > > On 11/6/17 7:06 PM, Guozhang Wang wrote: > > > > > > A quick question: I think we do not yet have the `list consumer > > > groups` > > > > > > func as in the old AdminClient. Without this `describe group` > given > > > the > > > > > > group id would not be very useful. Could you include this as well > > in > > > > your > > > > > > KIP? More specifically, you can look at > kafka.admin.AdminClientfor > > > more > > > > > > details on the APIs. > > > > > > > > > > > > > > > > > > Guozhang > > > > > > > > > > > > On Mon, Nov 6, 2017 at 7:22 AM, Ted Yu <yuzhih...@gmail.com> > > wrote: > > > > > > > > > > > >> Please fill out Discussion thread and JIRA fields. > > > > > >> > > > > > >> Thanks > > > > > >> > > > > > >> On Mon, Nov 6, 2017 at 2:02 AM, Tom Bentley < > > t.j.bent...@gmail.com> > > > > > wrote: > > > > > >> > > > > > >>> Hi Jorge, > > > > > >>> > > > > > >>> Thanks for the KIP. A few initial comments: > > > > > >>> > > > > > >>> 1. The AdminClient doesn't have any API like > > `listConsumerGroups()` > > > > > >>> currently, so in general how does a client know the group ids > it > > is > > > > > >>> interested in? > > > > > >>> 2. Could you fill in the API of DescribeConsumerGroupResult, > just > > > so > > > > > >>> everyone knows exactly what being proposed. > > > > > >>> 3. Can you describe the ConsumerGroupDescription class? > > > > > >>> 4. Probably worth mentioning that this will use > > > > > >>> DescribeGroupsRequest/Response, and also enumerating the error > > > codes > > > > > >> that > > > > > >>> can return (or, equivalently, enumerate the exceptions throw > from > > > the > > > > > >>> futures obtained from the DescribeConsumerGroupResult). > > > > > >>> > > > > > >>> Cheers, > > > > > >>> > > > > > >>> Tom > > > > > >>> > > > > > >>> On 6 November 2017 at 08:19, Jorge Esteban Quilcate Otoya < > > > > > >>> quilcate.jo...@gmail.com> wrote: > > > > > >>> > > > > > >>>> Hi everyone, > > > > > >>>> > > > > > >>>> I would like to start a discussion on KIP-222 [1] based on > issue > > > > [2]. > > > > > >>>> > > > > > >>>> Looking forward to feedback. > > > > > >>>> > > > > > >>>> [1] > > > > > >>>> https://cwiki.apache.org/confluence/pages/viewpage. > > > > > >>> action?pageId=74686265 > > > > > >>>> [2] https://issues.apache.org/jira/browse/KAFKA-6058 > > > > > >>>> > > > > > >>>> Cheers, > > > > > >>>> Jorge. > > > > > >>>> > > > > > >>> > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > -- Guozhang > > > > > > > > > > -- -- Guozhang
