You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by "Matthias J. Sax" <ma...@confluent.io> on 2017/12/18 22:38:01 UTC
[VOTE] KIP-243: Make ProducerConfig and ConsumerConfig constructors
public
Hi,
I want to propose the following KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-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
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Thanks for the feedback.
I updated the KIP and PR accordingly.
-Matthias
On 1/2/18 9:18 PM, Ewen Cheslack-Postava wrote:
> Builders have historically seen some resistance in the project (by some
> notable original authors...) but they've been increasingly making their way
> in -- SchemaBuilder in Connect, I believe some small streams APIs,
> AdminClient stuff. The general trend seems to be towards more fluent APIs.
>
> Regarding the KIP, I'm not sure why the APIs are currently Map<?, ?>, but
> this seems wrong. We have a mess of old Properties compatibility (which we
> seem to have dragged into the new producer/consumer APIs), but we could at
> least make these Map<String, ?>. (I prefer just doing Map<String, String>
> since we go through a parse phase regardless of the types, but didn't
> manage to make that happen within Connect.)
>
> Otherwise, the general idea seems fine to me.
>
> -Ewen
>
> On Thu, Dec 21, 2017 at 2:28 PM, Jason Gustafson <ja...@confluent.io> wrote:
>
>> I didn't sense much resistance in that thread, just an effort to keep the
>> streams and core client config APIs consistent ;).
>>
>> I'd prefer seeing a KIP for a more general improvement, but this change
>> seems harmless and improves consistency between the clients, so +1 from me.
>>
>> -Jason
>>
>> On Thu, Dec 21, 2017 at 11:19 AM, Matthias J. Sax <ma...@confluent.io>
>> wrote:
>>
>>> 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 <bb...@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 <yu...@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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by Ewen Cheslack-Postava <ew...@confluent.io>.
Builders have historically seen some resistance in the project (by some
notable original authors...) but they've been increasingly making their way
in -- SchemaBuilder in Connect, I believe some small streams APIs,
AdminClient stuff. The general trend seems to be towards more fluent APIs.
Regarding the KIP, I'm not sure why the APIs are currently Map<?, ?>, but
this seems wrong. We have a mess of old Properties compatibility (which we
seem to have dragged into the new producer/consumer APIs), but we could at
least make these Map<String, ?>. (I prefer just doing Map<String, String>
since we go through a parse phase regardless of the types, but didn't
manage to make that happen within Connect.)
Otherwise, the general idea seems fine to me.
-Ewen
On Thu, Dec 21, 2017 at 2:28 PM, Jason Gustafson <ja...@confluent.io> wrote:
> I didn't sense much resistance in that thread, just an effort to keep the
> streams and core client config APIs consistent ;).
>
> I'd prefer seeing a KIP for a more general improvement, but this change
> seems harmless and improves consistency between the clients, so +1 from me.
>
> -Jason
>
> On Thu, Dec 21, 2017 at 11:19 AM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
> > 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 <bb...@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 <yu...@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
> > >>>>>
> > >>>>
> > >>>
> > >>
> > >>
> > >
> >
> >
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by Jason Gustafson <ja...@confluent.io>.
I didn't sense much resistance in that thread, just an effort to keep the
streams and core client config APIs consistent ;).
I'd prefer seeing a KIP for a more general improvement, but this change
seems harmless and improves consistency between the clients, so +1 from me.
-Jason
On Thu, Dec 21, 2017 at 11:19 AM, Matthias J. Sax <ma...@confluent.io>
wrote:
> 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 <ma...@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 <bb...@gmail.com>
> wrote:
> >>>
> >>>> +1
> >>>>
> >>>> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wa...@gmail.com>
> >>>> wrote:
> >>>>
> >>>>> +1
> >>>>>
> >>>>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t....@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 <yu...@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
> >>>>>
> >>>>
> >>>
> >>
> >>
> >
>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by "Matthias J. Sax" <ma...@confluent.io>.
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 <ma...@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 <bb...@gmail.com> wrote:
>>>
>>>> +1
>>>>
>>>> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wa...@gmail.com>
>>>> wrote:
>>>>
>>>>> +1
>>>>>
>>>>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t....@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 <yu...@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
>>>>>
>>>>
>>>
>>
>>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by "Matthias J. Sax" <ma...@confluent.io>.
+1
I am closing this vote as accepted with 5 non-binding +1 (Ted, Vahid,
Tom, Bill, Matthias) and 3 binding +1 (Guozhang, Jason, Ewen) votes.
Thanks a lot.
-Matthias
On 1/8/18 1:29 PM, Matthias J. Sax wrote:
> We cannot simply change to <String,?> without breaking a lot of code and
> making it a nightmare to fix it up... :(
>
> I will leave the VOTE open for some more time if people still want to
> comment. Otherwise, we would have 3 bindings vote now.
>
>
> -Matthias
>
> On 1/8/18 10:58 AM, Ewen Cheslack-Postava wrote:
>> +1 (binding), though I still think the Map should be <String, ?> instead of
>> <?, ?>.
>>
>> I also think its better to just expose the defaults as constants on the
>> class. Apparently there was discussion of this before and the concern is
>> that people write code that rely on the default configs and we might break
>> their code if we change it. I don't really buy this as using the constant
>> allows you to to symbolically reference the value rather than making your
>> own copy of it. Usually if we change a default like that there is an
>> important reason why and having the old copied value might be worse than
>> having the value change out from under you. Having the defaults explicitly
>> exposed can also be helpful when writing tests sometimes.
>>
>> -Ewen
>>
>> On Wed, Jan 3, 2018 at 9:30 AM, Colin McCabe <cm...@apache.org> wrote:
>>
>>> On Thu, Dec 21, 2017, at 10:28, 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.
>>>
>>> Yeah, that would be good. The builder idea would definitely make it a lot
>>> easier to configure clients programmatically.
>>>
>>> I do wonder if there are some cross-version compatibility issues here. If
>>> there's any configuration that needs to be set by the client, but then
>>> propagated to the broker to be applied, the validation of that
>>> configuration really needs to be done by the broker itself. The client
>>> code doesn't know the broker version, so it can't validate these configs.
>>> One example is topic configurations (although those are not set by
>>> ProducerConfig). I'm not sure how big of an issue this is with our current
>>> configurations.
>>>
>>> Another problem here is that all these builder functions become API, and
>>> cannot easily be changed. So if we want to change a configuration key that
>>> formerly accepted an int to accept a long, it will be difficult to do
>>> that. We would have to add a new function with a separate name.
>>>
>>> best,
>>> Colin
>>>
>>>
>>>>
>>>> What do you think?
>>>>
>>>> -Jason
>>>>
>>>> On Wed, Dec 20, 2017 at 5:59 PM, Matthias J. Sax <ma...@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 <bb...@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 <yu...@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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>
>>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by "Matthias J. Sax" <ma...@confluent.io>.
One follow up, addressing Ewen's comment:
It seems an older KIP update was not reflected (maybe I forgot to hit
save or only though I updated the KIP -- sorry for that). The new API
will be
> public ProducerConfig(Properties props);
> public ProducerConfig(Map<String, Object> props);
> public ConsumerConfig(Properties props);
> public ConsumerConfig(Map<String, Object> props);
-Matthias
On 1/8/18 1:29 PM, Matthias J. Sax wrote:
> We cannot simply change to <String,?> without breaking a lot of code and
> making it a nightmare to fix it up... :(
>
> I will leave the VOTE open for some more time if people still want to
> comment. Otherwise, we would have 3 bindings vote now.
>
>
> -Matthias
>
> On 1/8/18 10:58 AM, Ewen Cheslack-Postava wrote:
>> +1 (binding), though I still think the Map should be <String, ?> instead of
>> <?, ?>.
>>
>> I also think its better to just expose the defaults as constants on the
>> class. Apparently there was discussion of this before and the concern is
>> that people write code that rely on the default configs and we might break
>> their code if we change it. I don't really buy this as using the constant
>> allows you to to symbolically reference the value rather than making your
>> own copy of it. Usually if we change a default like that there is an
>> important reason why and having the old copied value might be worse than
>> having the value change out from under you. Having the defaults explicitly
>> exposed can also be helpful when writing tests sometimes.
>>
>> -Ewen
>>
>> On Wed, Jan 3, 2018 at 9:30 AM, Colin McCabe <cm...@apache.org> wrote:
>>
>>> On Thu, Dec 21, 2017, at 10:28, 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.
>>>
>>> Yeah, that would be good. The builder idea would definitely make it a lot
>>> easier to configure clients programmatically.
>>>
>>> I do wonder if there are some cross-version compatibility issues here. If
>>> there's any configuration that needs to be set by the client, but then
>>> propagated to the broker to be applied, the validation of that
>>> configuration really needs to be done by the broker itself. The client
>>> code doesn't know the broker version, so it can't validate these configs.
>>> One example is topic configurations (although those are not set by
>>> ProducerConfig). I'm not sure how big of an issue this is with our current
>>> configurations.
>>>
>>> Another problem here is that all these builder functions become API, and
>>> cannot easily be changed. So if we want to change a configuration key that
>>> formerly accepted an int to accept a long, it will be difficult to do
>>> that. We would have to add a new function with a separate name.
>>>
>>> best,
>>> Colin
>>>
>>>
>>>>
>>>> What do you think?
>>>>
>>>> -Jason
>>>>
>>>> On Wed, Dec 20, 2017 at 5:59 PM, Matthias J. Sax <ma...@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 <bb...@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 <yu...@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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>
>>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by "Matthias J. Sax" <ma...@confluent.io>.
We cannot simply change to <String,?> without breaking a lot of code and
making it a nightmare to fix it up... :(
I will leave the VOTE open for some more time if people still want to
comment. Otherwise, we would have 3 bindings vote now.
-Matthias
On 1/8/18 10:58 AM, Ewen Cheslack-Postava wrote:
> +1 (binding), though I still think the Map should be <String, ?> instead of
> <?, ?>.
>
> I also think its better to just expose the defaults as constants on the
> class. Apparently there was discussion of this before and the concern is
> that people write code that rely on the default configs and we might break
> their code if we change it. I don't really buy this as using the constant
> allows you to to symbolically reference the value rather than making your
> own copy of it. Usually if we change a default like that there is an
> important reason why and having the old copied value might be worse than
> having the value change out from under you. Having the defaults explicitly
> exposed can also be helpful when writing tests sometimes.
>
> -Ewen
>
> On Wed, Jan 3, 2018 at 9:30 AM, Colin McCabe <cm...@apache.org> wrote:
>
>> On Thu, Dec 21, 2017, at 10:28, 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.
>>
>> Yeah, that would be good. The builder idea would definitely make it a lot
>> easier to configure clients programmatically.
>>
>> I do wonder if there are some cross-version compatibility issues here. If
>> there's any configuration that needs to be set by the client, but then
>> propagated to the broker to be applied, the validation of that
>> configuration really needs to be done by the broker itself. The client
>> code doesn't know the broker version, so it can't validate these configs.
>> One example is topic configurations (although those are not set by
>> ProducerConfig). I'm not sure how big of an issue this is with our current
>> configurations.
>>
>> Another problem here is that all these builder functions become API, and
>> cannot easily be changed. So if we want to change a configuration key that
>> formerly accepted an int to accept a long, it will be difficult to do
>> that. We would have to add a new function with a separate name.
>>
>> best,
>> Colin
>>
>>
>>>
>>> What do you think?
>>>
>>> -Jason
>>>
>>> On Wed, Dec 20, 2017 at 5:59 PM, Matthias J. Sax <ma...@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 <bb...@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 <yu...@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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by Ewen Cheslack-Postava <ew...@confluent.io>.
+1 (binding), though I still think the Map should be <String, ?> instead of
<?, ?>.
I also think its better to just expose the defaults as constants on the
class. Apparently there was discussion of this before and the concern is
that people write code that rely on the default configs and we might break
their code if we change it. I don't really buy this as using the constant
allows you to to symbolically reference the value rather than making your
own copy of it. Usually if we change a default like that there is an
important reason why and having the old copied value might be worse than
having the value change out from under you. Having the defaults explicitly
exposed can also be helpful when writing tests sometimes.
-Ewen
On Wed, Jan 3, 2018 at 9:30 AM, Colin McCabe <cm...@apache.org> wrote:
> On Thu, Dec 21, 2017, at 10:28, 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.
>
> Yeah, that would be good. The builder idea would definitely make it a lot
> easier to configure clients programmatically.
>
> I do wonder if there are some cross-version compatibility issues here. If
> there's any configuration that needs to be set by the client, but then
> propagated to the broker to be applied, the validation of that
> configuration really needs to be done by the broker itself. The client
> code doesn't know the broker version, so it can't validate these configs.
> One example is topic configurations (although those are not set by
> ProducerConfig). I'm not sure how big of an issue this is with our current
> configurations.
>
> Another problem here is that all these builder functions become API, and
> cannot easily be changed. So if we want to change a configuration key that
> formerly accepted an int to accept a long, it will be difficult to do
> that. We would have to add a new function with a separate name.
>
> best,
> Colin
>
>
> >
> > What do you think?
> >
> > -Jason
> >
> > On Wed, Dec 20, 2017 at 5:59 PM, Matthias J. Sax <ma...@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 <bb...@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 <yu...@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
> > > >>>
> > > >>
> > > >
> > >
> > >
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig constructors
public
Posted by Colin McCabe <cm...@apache.org>.
On Thu, Dec 21, 2017, at 10:28, 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.
Yeah, that would be good. The builder idea would definitely make it a lot easier to configure clients programmatically.
I do wonder if there are some cross-version compatibility issues here. If there's any configuration that needs to be set by the client, but then propagated to the broker to be applied, the validation of that configuration really needs to be done by the broker itself. The client code doesn't know the broker version, so it can't validate these configs. One example is topic configurations (although those are not set by ProducerConfig). I'm not sure how big of an issue this is with our current configurations.
Another problem here is that all these builder functions become API, and cannot easily be changed. So if we want to change a configuration key that formerly accepted an int to accept a long, it will be difficult to do that. We would have to add a new function with a separate name.
best,
Colin
>
> What do you think?
>
> -Jason
>
> On Wed, Dec 20, 2017 at 5:59 PM, Matthias J. Sax <ma...@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 <bb...@gmail.com> wrote:
> > >
> > >> +1
> > >>
> > >> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wa...@gmail.com>
> > >> wrote:
> > >>
> > >>> +1
> > >>>
> > >>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t....@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 <yu...@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
> > >>>
> > >>
> > >
> >
> >
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by Jason Gustafson <ja...@confluent.io>.
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 <ma...@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 <bb...@gmail.com> wrote:
> >
> >> +1
> >>
> >> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wa...@gmail.com>
> >> wrote:
> >>
> >>> +1
> >>>
> >>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t....@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 <yu...@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
> >>>
> >>
> >
>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by "Matthias J. Sax" <ma...@confluent.io>.
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 <bb...@gmail.com> wrote:
>
>> +1
>>
>> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wa...@gmail.com>
>> wrote:
>>
>>> +1
>>>
>>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t....@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 <yu...@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
>>>
>>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by "Matthias J. Sax" <ma...@confluent.io>.
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 <bb...@gmail.com> wrote:
>
>> +1
>>
>> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wa...@gmail.com>
>> wrote:
>>
>>> +1
>>>
>>> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t....@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 <yu...@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
>>>
>>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by Jason Gustafson <ja...@confluent.io>.
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 <bb...@gmail.com> wrote:
> +1
>
> On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wa...@gmail.com>
> wrote:
>
> > +1
> >
> > On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t....@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 <yu...@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
> >
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by Bill Bejeck <bb...@gmail.com>.
+1
On Tue, Dec 19, 2017 at 12:09 PM, Guozhang Wang <wa...@gmail.com> wrote:
> +1
>
> On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t....@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 <yu...@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
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by Guozhang Wang <wa...@gmail.com>.
+1
On Tue, Dec 19, 2017 at 1:49 AM, Tom Bentley <t....@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 <yu...@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 <ma...@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
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by Tom Bentley <t....@gmail.com>.
+1
On 18 December 2017 at 23:28, Vahid S Hashemian <va...@us.ibm.com>
wrote:
> +1
>
> Thanks for the KIP.
>
> --Vahid
>
>
>
> From: Ted Yu <yu...@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 <ma...@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
> >
> >
>
>
>
>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig constructors
public
Posted by Vahid S Hashemian <va...@us.ibm.com>.
+1
Thanks for the KIP.
--Vahid
From: Ted Yu <yu...@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 <ma...@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
>
>
Re: [VOTE] KIP-243: Make ProducerConfig and ConsumerConfig
constructors public
Posted by Ted Yu <yu...@gmail.com>.
+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 <ma...@confluent.io>
wrote:
> Hi,
>
> I want to propose the following KIP:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 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
>
>