I personally love the builder pattern idea. There was some push back in the past though from some people.
cf https://issues.apache.org/jira/browse/KAFKA-4436 Happy to propose the builder pattern but than we should have a proper DISCUSS thread. Maybe we do this as a follow up and just do this KIP as-is? -Matthias On 12/21/17 10:28 AM, Jason Gustafson wrote: > Hey Matthias, > > Let me suggest an alternative. As you have mentioned, these config classes > do not give users much benefit currently. Maybe we change that? I think > many users would appreciate having a builder for configuration since it > provides type safety and is generally a much friendlier pattern to work > with programmatically. Users could then do something like this: > > ConsumerConfig config = ConsumerConfig.newBuilder() > .setBootstrapServers("localhost:9092") > .setGroupId("group") > .setRequestTimeout(15, TimeUnit.SECONDS) > .build(); > > Consumer consumer = new KafkaConsumer(config); > > An additional benefit of this is that it gives us a better way to expose > config deprecations. In any case, it would make it less odd to expose the > public constructor without giving users anything useful to do with the > class. > > What do you think? > > -Jason > > On Wed, Dec 20, 2017 at 5:59 PM, Matthias J. Sax <matth...@confluent.io> > wrote: > >> It's tailored for internal usage. I think client constructors don't >> benefit from accepting those config objects. We just want to be able to >> access the default values for certain parameters. >> >> From a user point of view, it's actually boiler plate code if you pass >> in a config object instead of a plain Properties object because the >> config object itself is immutable. >> >> I actually create a JIRA to remove the constructors from KafkaStreams >> that do accept StreamsConfig for exact this reason: >> https://issues.apache.org/jira/browse/KAFKA-6386 >> >> >> -Matthias >> >> >> On 12/20/17 3:33 PM, Jason Gustafson wrote: >>> Hi Matthias, >>> >>> Isn't it a little weird to make these constructors public but not also >>> expose the corresponding client constructors that use them? >>> >>> -Jason >>> >>> On Tue, Dec 19, 2017 at 9:30 AM, Bill Bejeck <bbej...@gmail.com> wrote: >>> >>>> +1 >>>> >>>> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wangg...@gmail.com> >>>> wrote: >>>> >>>>> +1 >>>>> >>>>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t.j.bent...@gmail.com> >>>>> wrote: >>>>> >>>>>> +1 >>>>>> >>>>>> On 18 December 2017 at 23:28, Vahid S Hashemian < >>>>> vahidhashem...@us.ibm.com >>>>>>> >>>>>> wrote: >>>>>> >>>>>>> +1 >>>>>>> >>>>>>> Thanks for the KIP. >>>>>>> >>>>>>> --Vahid >>>>>>> >>>>>>> >>>>>>> >>>>>>> From: Ted Yu <yuzhih...@gmail.com> >>>>>>> To: dev@kafka.apache.org >>>>>>> Date: 12/18/2017 02:45 PM >>>>>>> Subject: Re: [VOTE] KIP-243: Make ProducerConfig and >>>>>> ConsumerConfig >>>>>>> constructors public >>>>>>> >>>>>>> >>>>>>> >>>>>>> +1 >>>>>>> >>>>>>> nit: via "copy and past" an 'e' is missing at the end. >>>>>>> >>>>>>> On Mon, Dec 18, 2017 at 2:38 PM, Matthias J. Sax < >>>>> matth...@confluent.io> >>>>>>> wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> I want to propose the following KIP: >>>>>>>> >>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki. >>>>>>> apache.org_confluence_display_KAFKA_KIP-2D&d=DwIBaQ&c=jf_ >>>>>>> iaSHvJObTbx-siA1ZOg&r=Q_itwloTQj3_xUKl7Nzswo6KE4Nj- >>>>>>> kjJc7uSVcviKUc&m=JToRX4-HeVsRoOekIz18ht-YLMe-T21MttZTgbxB4ag&s= >>>>>>> 6aZjPCc9e00raokVPKvx1BxwDOHyCuKNgtBXPMeoHy4&e= >>>>>>> >>>>>>>> 243%3A+Make+ProducerConfig+and+ConsumerConfig+constructors+public >>>>>>>> >>>>>>>> >>>>>>>> This is a rather straight forward change, thus I skip the DISCUSS >>>>>>>> thread and call for a vote immediately. >>>>>>>> >>>>>>>> >>>>>>>> -Matthias >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> -- Guozhang >>>>> >>>> >>> >> >> >
signature.asc
Description: OpenPGP digital signature