kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthias J. Sax" <matth...@confluent.io>
Subject Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig constructors public
Date Thu, 21 Dec 2017 19:19:57 GMT
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 <matthias@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 <bbejeck@gmail.com> wrote:
>>>
>>>> +1
>>>>
>>>> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wangguoz@gmail.com>
>>>> wrote:
>>>>
>>>>> +1
>>>>>
>>>>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t.j.bentley@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> +1
>>>>>>
>>>>>> On 18 December 2017 at 23:28, Vahid S Hashemian <
>>>>> vahidhashemian@us.ibm.com
>>>>>>>
>>>>>> wrote:
>>>>>>
>>>>>>> +1
>>>>>>>
>>>>>>> Thanks for the KIP.
>>>>>>>
>>>>>>> --Vahid
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> From:   Ted Yu <yuzhihong@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 <
>>>>> matthias@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
>>>>>
>>>>
>>>
>>
>>
> 


Mime
View raw message