Hello everyone, I'm ending this KIP vote as accepted with 4 binding +1s (John, Bruno, David, Luke).
The wiki page has been updated and the corresponding code merged to trunk. Thanks! Guozhang On Wed, Jul 6, 2022 at 7:02 PM Luke Chen <show...@gmail.com> wrote: > Hi Guozhang, > > Removing the script-extention from the KIP is good to me. > +1 from me, too. > > Thank you. > Luke > > On Thu, Jul 7, 2022 at 7:39 AM Guozhang Wang <wangg...@gmail.com> wrote: > > > 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 > > > -- -- Guozhang
