Re: Discussion on KIP-670 : Add ConsumerGroupCommand to delete static members

[David Jacot <dj...@confluent.io.INVALID>]

Hi Sandeep,

Thanks for the KIP. Overall, your proposal seems to be a useful
addition to the command line tool. I made a first pass on it and I
have a few comments:

01. Adding `removeMembersFromConsumerGroup(String groupId,
Collection<String> memberIds)` does not seem necessary because you can
already specify members in the
`RemoveMembersFromConsumerGroupOptions`.

02. Note that you can either remove all members in the group or remove
members by specifying their instance id. It would be good to be clear
about this in the KIP.

03. Could you add a Public Interfaces section and describe the new
command line arguments in there? It would be also good to add a few
examples.

04. Is it possible to pass multiple members or only one? It would be
good to make this clear in the KIP.

05. I would remove the code from the KIP. It is not required. The KIP
should focus on the motivation and the public interfaces.

Best,
David

On Thu, Sep 3, 2020 at 7:22 PM Walker Carlson <wc...@confluent.io> wrote:
>
> Hello Sandeep,
>
> Reading through your kip it seems like a good idea and pretty straight
> forward. So I have no problems with this proposal.
>
> Thanks for the Kip,
>
> Walker
>
>
> On Thu, Sep 3, 2020 at 8:28 AM Sandeep Kumar <sn...@gmail.com> wrote:
>
> > Hi All,
> >
> > I am new to the Kafka contribution community. I have picked up a jira
> > ticket https://issues.apache.org/jira/browse/KAFKA-9440 which requires
> > KIP.
> >
> > I have submitted KIP for it
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-670%3A+Add+ConsumerGroupCommand+to+delete+static+members
> >
> > I am proposing that we add a new option (--remove-members) from consumer
> > group via CLI.
> >
> > I'd really appreciate your feedback on the proposal.
> >
> > Thanks and Regards,
> > Sandeep
> >

Re: Discussion on KIP-670 : Add ConsumerGroupCommand to delete static members

[Kirk True <ki...@kirktrue.pro>]

Hi Sandeep,

Thanks for the KIP!

Comments/questions:

1. Looking at the existing code, it does seem that the existing Admin APIs are sufficient, such that this can be implemented by augmenting the ConsumerGroupCommand directly. This cuts down on a lot of code and makes it easier to adopt in non-Java clients. If a separate method turns out to not be needed, perhaps you can mention that in the "Rejected Alternatives" section?

2. What happens if the user provides both of the --all-members and --member arguments when invoking --remove-members? Is that something we want to prohibit by adding a check to checkArgs()? What happens i

3. What happens if the member ID provide is incorrect (either a typo or a race condition)? Could you maybe provide a sentence of two on that? Even if handling that is outside of the code for this KIP, stating that is helpful, too.

Thanks,
Kirk

On Fri, Dec 16, 2022, at 8:31 AM, David Jacot wrote:
> Hi Sandeep,
> 
> Thanks for the KIP. Overall, your proposal seems to be a useful
> addition to the command line tool. I made a first pass on it and I
> have a few comments:
> 
> 01. Adding `removeMembersFromConsumerGroup(String groupId,
> Collection<String> memberIds)` does not seem necessary because you can
> already specify members in the
> `RemoveMembersFromConsumerGroupOptions`.
> 
> 02. Note that you can either remove all members in the group or remove
> members by specifying their instance id. It would be good to be clear
> about this in the KIP.
> 
> 03. Could you add a Public Interfaces section and describe the new
> command line arguments in there? It would be also good to add a few
> examples.
> 
> 04. Is it possible to pass multiple members or only one? It would be
> good to make this clear in the KIP.
> 
> 05. I would remove the code from the KIP. It is not required. The KIP
> should focus on the motivation and the public interfaces.
> 
> Best,
> David
> 
> On Thu, Sep 3, 2020 at 7:22 PM Walker Carlson <wc...@confluent.io> wrote:
> >
> > Hello Sandeep,
> >
> > Reading through your kip it seems like a good idea and pretty straight
> > forward. So I have no problems with this proposal.
> >
> > Thanks for the Kip,
> >
> > Walker
> >
> >
> > On Thu, Sep 3, 2020 at 8:28 AM Sandeep Kumar <sn...@gmail.com> wrote:
> >
> > > Hi All,
> > >
> > > I am new to the Kafka contribution community. I have picked up a jira
> > > ticket https://issues.apache.org/jira/browse/KAFKA-9440 which requires
> > > KIP.
> > >
> > > I have submitted KIP for it
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-670%3A+Add+ConsumerGroupCommand+to+delete+static+members
> > >
> > > I am proposing that we add a new option (--remove-members) from consumer
> > > group via CLI.
> > >
> > > I'd really appreciate your feedback on the proposal.
> > >
> > > Thanks and Regards,
> > > Sandeep
> > >
>