Hi Sagar,
Yes, I think I misunderstood your KIP.
I have no other comments, then.
Thank you.
Luke
On Tue, May 31, 2022 at 12:46 PM Sagar wrote:
> Hey Luke,
>
> Actually the addMetric still throws IllegalArgumentException. The new
> method that is being introduced as part of the KIP doesn't thro
Hey Luke,
Actually the addMetric still throws IllegalArgumentException. The new
method that is being introduced as part of the KIP doesn't throw any
exceptions. Also, the KIP was an afterthought, which came up during the PR
review of this: https://github.com/apache/kafka/pull/12121
Let me know if
Hi Sagar,
Thanks for the KIP.
In the `Compatibility` section, you mentioned:
*The changes are backward compatible and needs no deprecation/migration.*
But as you said, we won't throw `IllegalArgumentException` in `addMetric`
method. Isn't that the compatibility issue that should be put in
compat
Hi Sagar,
Thank you for the updates!
2. Great if we just need one overload!
Best,
Bruno
On 24.05.22 18:30, Sagar wrote:
Hi Bruno,
Thanks for your comments.
1) I updated the header in the public interfaces section.
2) Well, the overloads were there initially but then it was suggested to
remo
Hi Bruno,
Thanks for your comments.
1) I updated the header in the public interfaces section.
2) Well, the overloads were there initially but then it was suggested to
remove them during PR review:
https://github.com/apache/kafka/pull/12121#discussion_r875156714
3) I updated the KIP with the funct
Sagar, Thanks for the KIP!
1. Could you please add the class to which you want to add the method in
section "Public Interfaces"? Ideally, in the header of the code part
that contains "prefixScan" currently.
2. Do you also plan to add overloads for metricOrElseCreate() similar as
for addMetr
Thanks Guozhang. I made the edit. Let me open a VOTE thread.
Thanks!
Sagar.
On Mon, May 23, 2022 at 10:19 PM Guozhang Wang wrote:
> Thanks Sagar,
>
> The KIP looks good to me overall, I just have a minor comment on the
> motivation of it: "The Metrics registry is often used by concurrent
> th
Thanks Sagar,
The KIP looks good to me overall, I just have a minor comment on the
motivation of it: "The Metrics registry is often used by concurrent
threads" -> maybe we can be more clearer, by saying that "Concurrent thread
may try to access the Metrics registry to create the same, instance-l
Hi All,
I would like to start a discussion thread on the following KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-843%3A+Adding+metricOrElseCreate+method+to+Metrics
PS: Note that I sent another email without the KIP number in the
subject line. Plz use this email for discussion and ig
