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* > > > > > > >
