Hi Jason,
Sorry for the late reply too. All proposed/dismissed changes done. Please review the latest PR updates: https://github.com/apache/kafka/pull/5726/ #1. Done. Backwards compatibility with old csv format is preserved. #2. Not. Considered not to omit the GROUP column for `--describe` output of a single `--group` since this is less important from a compatibility perspective. #3. Done. All tables of --describe output now start from the GROUP column. #4. Not. Can be implemented as a separate feature. Do you confirm changes? Thank you, Alex Dunayevsky >Hi Alex, > >Sorry for the late reply. Your message didn't get attached to the main >thread and I missed it. > >#1. Yeah, I think we should continue to accept the old csv format. >Compatibility is important for all public APIs. >#2. I think this is less important from a compatibility perspective. On the >one hand, it makes the output compatible with currently supported usage. On >the other, it makes it more annoying to write tools which invoke this >command because they need to treat the single group case separately. I'm >probably leaning toward not doing this one, but I don't have a strong >opinion. >#3. To clarify, my suggestion was to put the group id first. I think Vahid >was in agreement. From your comment, it sounds like you agree as well? >#4. I agree supporting regex can be left for future work. > >Thanks, >Jason > >On Mon, Nov 5, 2018 at 7:55 AM Alex D <a.a.dunayev...@gmail.com> wrote: > >>Hello guys, >> >>Thank you for your suggestions! >>I've made a short resume of all suggestions proposed for further >>possible code corrections. >>Since not all opinions match, let's review once again and decide. >> >>#1. Support old csv format. Proposed by Jason. >> Yes: Jason, Vahid >> >> If backwards compatibility is important for this specific (and, I >>believe, infrequent) case, ready to make corrections. Final word? >> >>#2. Do not show group name for `--describe` output in case a single >>`--group` is specified. Proposed by Jason. >> Yes: Jason >> >> Alternatively, the user will always expect the output to be the >>same >>for any `--describe` query. Ready to make corrections if this is >>important. Final word? >> >>#3. GROUP column should not be the first in the row. Proposed by Jason. >> Yes: Jason >> No: Vahid >> >> For the group offset configuration, the group entity appears to be >>the top priority and starting a table with a GROUP column makes more >>sense, I believe. Plus, it's quicker and easier to spot to which group >>the offsets belong to. >> Apply corrections or leave as is? >> >>#4. Single regex vs multiple `--group` flags. Proposed by eazama.. >> >> There are a few reasons behind this. Firstly, there are no rules >>for >>defining group names unlike for topic names that have their own >>validation routine according to a simple regex. Group names may >>contain any symbols possible and simply splitting them by comma won't >>work, at least without using escape characters maybe. Secondly, >>repetition of the `--group` flag had already been implemented for the >>group deletion logic and we don't not want to break the backwards >>compatibility. Finally, visually, it's a bit easier to read and >>compose a long query with a large number of groups than throwing >>everything into one very long string. >> >>#5. Valid scenario where we would want to delete all consumer groups. >>Asked by Vahid. >> >> There should be one, I believe ;) Already received a few requests >>from colleagues. >> >># KIP approvals: >> Suman: +1 >> >>> Sat, 20 Oct 2018 17:10:16 GMT, <eazama...@gmail.com> wrote: >>> Is there a reason for using multiple --group flags over having it accept >>a regex? >>> >>> The topics command currently accepts a regex for most operations and >>doesn't support using >>> multiple topics flags. It seems like it would be better to take a more >>standardized approach >>> to providing this type of information. >>> >>> >>>> On Oct 19, 2018, at 10:28 AM, Suman B N <sumannew...@gmail.com> wrote: >>>> >>>> This eases debugging metadata information of consumer groups and >>offsets in >>>> case of client hungs which we have been facing frequently. >>>> +1 from me. Well done Alex! >>>> >>>> -Suman >>>> >>>> On Fri, Oct 19, 2018 at 8:36 PM Vahid Hashemian < >>vahid.hashem...@gmail.com> >>>> wrote: >>>> >>>>> Thanks for proposing the KIP. Looks good to me overall. >>>>> >>>>> I agree with Jason's suggestion that it would be best to keep the >>current >>>>> output format when a single '--group' is present. Because otherwise, >>there >>>>> would be an impact to users who rely on the current output format. >>Also, >>>>> starting with a GROUP column makes more sense to me. >>>>> >>>>> Also, and for my own info, is there a valid scenario where we would >>want to >>>>> delete all consumer groups? It sounds to me like a potentially >>dangerous >>>>> feature. I would imagine that it would help more with dev/test >>>>> environments, where we normally have a few groups (for which the >>repeating >>>>> '--group' option should work). >>>>> >>>>> Regards! >>>>> --Vahid >>>>> >>>>> On Thu, Oct 18, 2018 at 11:28 PM Jason Gustafson <ja...@confluent.io> >>>>> wrote: >>>>> >>>>>> Hi Alex, >>>>>> >>>>>> Thanks for the KIP. I think it makes sense, especially since most of >>the >>>>>> group apis are intended for batching anyway. >>>>>> >>>>>> The only questions I have are about compatibility. For example, the >>csv >>>>>> format for resetting offsets is changed, so will we continue to >>support >>>>> the >>>>>> old format? Also, if only one `--group` option is passed, do you think >>>>> it's >>>>>> worth leaving it the group column out of the `--describe` output? That >>>>>> would keep the describe output consistent with the current >>implementation >>>>>> for the current usage. Finally, and this is just a nitpick, but do you >>>>>> think it makes sense to put the group column first in the describe >>>>> output? >>>>>> >>>>>> Thanks, >>>>>> Jason >>>>>> >>>>>> >>>>>>> On Wed, Oct 3, 2018 at 7:11 AM, Alex D <a.a.dunayev...@gmail.com> >>wrote: >>>>>>> >>>>>>> Hello, friends! >>>>>>> >>>>>>> Welcome to the Multiple Consumer Group Management feature for >>>>>>> kafka-consumer-groups utility discussion thread. >>>>>>> >>>>>>> KIP is available here: >>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-379% >>>>>>> 3A+Multiple+Consumer+Group+Management >>>>>>> >>>>>>> Pull Request: https://github.com/apache/kafka/pull/5726 >>>>>>> >>>>>>> JIRA ticket: https://issues.apache.org/jira/browse/KAFKA-7471 >>>>>>> >>>>>>> What do you think? >>>>>>> >>>>>>> Thanks, >>>>>>> Alexander Dunayevsky >>>>>>> >>>>>> >>>>> >>>> >>>> >>>> -- >>>> *Suman* >>>> *OlaCabs* >>> >>> >>> >> >
