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


Reply via email to