Hello folks, I tried to implement the script-extension along with the unit test coverage on DescribeConsumerGroupTest, but it turns out more complicated than I anticipated due to the fact that we need to make sure the `require-stable` flag is only effective for describing consumers. This also makes me feeling that maybe my original thought of just adding that option for "--describe" may not be comprehensive and we may need to think through under which action the additional option should be allowed. So I would remove this part of the KIP and continue the voting thread.
I've updated the KIP addressing the renaming suggestion. And I will close this thread as accepted with three binding votes (John, David, Bruno) if I don't hear from you for more suggestions. Thanks, Guozhang On Mon, Jul 4, 2022 at 1:57 AM Bruno Cadonna <cado...@apache.org> wrote: > Hi, > > I would prefer to not include the script-extension into the KIP if you > you cannot commit to its implementation. I think partially implemented > KIPs make release management harder. If we can avoid implementing KIPs > partially, we should do it. > > I am +1 either way. I just wanted to bring this up. > > Best, > Bruno > > On 04.07.22 04:37, Luke Chen wrote: > > Hi Guozhang, > > > >> We can add it into this proposal though I could not commit to > implementing > > it myself with all the `DescribeConsumerGroupTest` coverage after it's > > accepted, instead I could add a JIRA ticket under this KIP for others > who's > > interested to chime in. What do you think? > > > > Sounds good to me. > > > > Thank you. > > Luke > > > > On Mon, Jul 4, 2022 at 3:44 AM Guozhang Wang <wangg...@gmail.com> wrote: > > > >> Thanks for folks for your input ! > >> > >> 1) I'm happy to change the setter names to be consistent with the > >> topicPartition ones. I used a different name for getter from setter as I > >> remember seeing some other options differentiating function names for > >> getter and setters, while some other options seem to be more on just > >> keeping the names the same. After getting your feedback I think it's > better > >> to do the same for both getter / setters. > >> > >> 2) For the kafka-consumer-group.sh tool, I looked at > >> ConsumerGroupCommand#describeGroups, and I think it's appropriate to add > >> it. I'm planning to add it in the shell tool as: > >> > >> ``` > >> "Require brokers to hold on returning unstable offsets (due to pending > >> transactions) but retry until timeout for stably committed offsets" > >> "Example: --bootstrap-server localhost:9092 --describe --group group1 > >> --offsets --require-stable" > >> ``` > >> > >> We can add it into this proposal though I could not commit to > implementing > >> it myself with all the `DescribeConsumerGroupTest` coverage after it's > >> accepted, instead I could add a JIRA ticket under this KIP for others > who's > >> interested to chime in. What do you think? > >> > >> > >> Guozhang > >> > >> > >> On Fri, Jul 1, 2022 at 1:18 AM David Jacot <dja...@confluent.io.invalid > > > >> wrote: > >> > >>> Hi Guozhang, > >>> > >>> Thanks for the KIP! > >>> > >>> I agree with Luke. `requireStable` seems more consistent. > >>> > >>> Regarding the kafka-consumer-group command line tool, I wonder if > >>> there is real value in doing it. We don't necessarily have to add all > >>> the options to it but we could if it is proven to be useful. Anyway, I > >>> would leave it for a future KIP. > >>> > >>> +1 (binding) > >>> > >>> Best, > >>> David > >>> > >>> On Fri, Jul 1, 2022 at 9:47 AM Bruno Cadonna <cado...@apache.org> > wrote: > >>>> > >>>> Hi Guozhang, > >>>> > >>>> thank you for the KIP! > >>>> > >>>> I do not have strong feelings about the naming of the getter, but I > >> tend > >>>> to agree with Luke. > >>>> > >>>> Regarding, the adaptation of the kafka-consumer-group.sh script, I am > >>>> fine if we leave that for a future KIP. > >>>> > >>>> +1 (binding) > >>>> > >>>> Best, > >>>> Bruno > >>>> > >>>> On 01.07.22 06:05, Luke Chen wrote: > >>>>> Hi Guozhang, > >>>>> > >>>>> Thanks for the KIP. > >>>>> Some comments: > >>>>> 1. I have the same question as Ziming, should we also add an option > >> in > >>>>> kafka-consumer-groups.sh in this KIP? > >>>>> Or you'd like to keep the current scope, and other people can create > >> a > >>>>> follow-up KIP to address the kafka-consumer-groups.sh script? > >>>>> 2. The setter method name: `shouldRequireStable` might need to rename > >>> to > >>>>> `requireStable` to be consistent with above `topicPartitions` > >>> getter/setter > >>>>> > >>>>> Thank you. > >>>>> Luke > >>>>> > >>>>> On Fri, Jul 1, 2022 at 11:17 AM John Roesler <vvcep...@apache.org> > >>> wrote: > >>>>> > >>>>>> Thanks for the KIP, Guozhang! > >>>>>> > >>>>>> I’m +1 (binding) > >>>>>> > >>>>>> -John > >>>>>> > >>>>>> On Thu, Jun 30, 2022, at 21:17, deng ziming wrote: > >>>>>>> Thanks for this KIP, > >>>>>>> we have a kafka-consumer-groups.sh shell which is based on the API > >>> you > >>>>>>> proposed to change, is it worth update it as well? > >>>>>>> > >>>>>>> -- > >>>>>>> Best, > >>>>>>> Ziming > >>>>>>> > >>>>>>>> On Jul 1, 2022, at 9:04 AM, Guozhang Wang <wangg...@gmail.com> > >>> wrote: > >>>>>>>> > >>>>>>>> Hello folks, > >>>>>>>> > >>>>>>>> I'd like to call out for a vote for the following KIP to expose > >> the > >>>>>>>> requireStable flag inside admin client's options as well: > >>>>>>>> > >>>>>>>> > >>>>>> > >>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-851%3A+Add+requireStable+flag+into+ListConsumerGroupOffsetsOptions > >>>>>>>> > >>>>>>>> Any feedback as well as your votes are welcome. > >>>>>>>> > >>>>>>>> -- Guozhang > >>>>>> > >>>>> > >>> > >> > >> > >> -- > >> -- Guozhang > >> > > > -- -- Guozhang
