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 <mj...@apache.org> wrote: > +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 `new > ProducerConfig` to begin with. If one creates a `KafkaProducer` they > pass the config via a `Map` or `Properties`, and the producer creates > `ProducerConfig` internally only. -- Thus, there is no need to make it > `public`. (To this end, I don't actually understand why there is public > `ProducerConfig` constructors to begin with -- sounds like a leaky > abstraction to me.) > > On the other hand, if a "third party" implements `Producer` interface to > ship their own producer implementation, they might want to create > `ProducerConfig` internally, so for them it's different, but still, they > don't need public access because they can extend `ProducerConfig`, too > for this case). -- To me, this falls into the category "simple thing > should be easy, and hard things should be possible). > > > -Matthias > > > On 11/3/23 6:06 AM, Ismael Juma wrote: > > 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 < > sop...@responsive.dev> > > wrote: > > > >> 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 > >> <https://github.com/apache/kafka/pull/14681#discussion_r1378591228> for > >> context). Since the change itself was already reviewed and approved I'm > >> skipping the discussion thread and bringing it to a vote right away, > but of > >> course I'm open to feedback and can create a discussion thread if there > is > >> need for it. > >> > >> The change itself is simply adding the `protected` modifier to the > >> ProducerConfig constructor that allows for silencing the config logging. > >> This just brings the ProducerConfig in alignment with the other client > >> configs, all of which already had this constructor as protected. > >> > >> KIP: > >> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-998%3A+Give+ProducerConfig%28props%2C+doLog%29+constructor+protected+access > >> PR: https://github.com/apache/kafka/pull/14681 > >> > >> Thanks! > >> Sophie > >> > > >
