Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

I am ok with the config proposal. For consumer/producer/admin client configs, I still don't understand why one would want to instantiate the object (for StreamsConfig with its complex "overwrite logic" of internal client configs, it makes more sense). But there is also no real harm done to

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

Thanks for the input Colin -- I think I'm with you in spirit, although the 2nd suggestion isn't viable as setting the log level to TRACE would mean silencing every instance of the config logging, which was not the intention here: we still want the configs to be logged once, when the client is

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

-1 If the goal is not to have so much config logging, let's add a configuration key that suppresses log4j in AbstractConfig. Then plugins can set that configuration if they want before invoking the superclass constructors. (Or in the interest of allowing people to be verbose if they want,

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

Sophie reads my mind well, but I also won't object if majority if people thinks it's desirable to have it public (it's does not really hurt to have them public). I just personally think, we should optimize for "end users" and they should not need it -- and thus, keeping the API surface area

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

No objections, I'm +1 ether way. On Fri, Nov 3, 2023, 18:50 Sophie Blee-Goldman wrote: > I am fine with making them public. Of course in that case we should also > change the corresponding constructors in ConsumerConfig, AdminConfig, and > StreamsConfig from protected to public as well, to be

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

I am fine with making them public. Of course in that case we should also change the corresponding constructors in ConsumerConfig, AdminConfig, and StreamsConfig from protected to public as well, to be consistent. But Matthias seems to feel that these should never be instantiated by a user and that

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

It seems wrong to require inheritance for this and we already have a public constructor. I would make both of them public. Ismael On Fri, Nov 3, 2023 at 10:47 AM Matthias J. Sax wrote: > +1 (binding) > > > About "why not public" question: > > I think we need to distinguish between "end users"

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

+1 (binding) About "why not public" question: I think we need to distinguish between "end users" who create a producer instance, and "external parties" that might implement their own `Producer` (or wrap/extend `KafkaProducer`). In the end, I would not expect an "end user" to actually call

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

Hi Sophie, I was trying to understand the goal of the change and it's not totally clear to me. If the goal is to allow third party applications to customize the logging behavior, why is the method protected instead of public? Ismael On Thu, Nov 2, 2023 at 9:55 PM Sophie Blee-Goldman wrote: >

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

Thanks for the KIP! +1 (binding) Best, Bruno On 11/3/23 7:55 AM, Chris Egerton wrote: +1 (binding) FWIW, I agree that this change should require a KIP. Gating upgrades of visibility from private or package-private to protected may be cumbersome, but it comes with the expectation that

Re: [VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

+1 (binding) FWIW, I agree that this change should require a KIP. Gating upgrades of visibility from private or package-private to protected may be cumbersome, but it comes with the expectation that downgrades in visibility for those same classes/methods will also be gated behind a KIP, which is

[VOTE] KIP-998: Give ProducerConfig(props, doLog) constructor protected access

Hey all, This is a trivial one-liner change that it was determined should go through a KIP during the PR review process (see this thread for context). Since the change itself was already reviewed and approved I'm skipping the