You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Bill Bejeck <bi...@confluent.io> on 2018/06/08 22:20:59 UTC
[DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
All,
I'd like to start the discussion for adding an overloaded method to
StreamsBuilder taking a java.util.Properties instance.
The KIP is located here :
https://cwiki.apache.org/confluence/display/KAFKA/KIP-312%3A+Add+Overloaded+StreamsBuilder+Build+Method+to+Accept+java.util.Properties
I look forward to your comments.
Thanks,
Bill
Re: [DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
Posted by Bill Bejeck <bb...@gmail.com>.
All, the discussion list for this proposed change has been quiet for a few
days.
If there are no changes or other proposals, I'll start a voting thread
later today.
Thanks,
Bill
On Wed, Jun 13, 2018 at 12:31 PM Guozhang Wang <wa...@gmail.com> wrote:
> Thanks for the explanation Bill. Makes sense to me.
>
> On Tue, Jun 12, 2018 at 9:21 AM, Bill Bejeck <bb...@gmail.com> wrote:
>
> > > Since there're only two values for the optional optimization config
> > > introduced by KAFKA-6935, I wonder the overloaded build method (with
> > Properties
> > > instance) would make the config unnecessary.
> >
> > Hi Ted, thanks for commenting. You raise a good point. Buy IMHO, yes we
> > still need the config as we want to give users the ability to turn
> > optimizations off/on explicitly and we haven't finalized anything
> > concerning how we'll pass in the parameters. Additionally, as we release
> > new versions, the config will give users the ability to choose to apply
> all
> > of the latest optimizations or stick with the previous version.
> >
> > Guozhang,
> >
> > > if we can hide this from the public API, to, e.g. add an additional
> > function
> > > in InternalTopologyBuilder of InternalStreamsBuilder (since in your
> > current
> > > working PR we're reusing InternalStreamsBuilder for the logical plan
> > > generation) which can then be called inside KafkaStreams
> constructors?
> >
> > I like the idea, but as I looked into it, there is an issue concerning
> the
> > fact that users can call Topology.describe() at any point. So with this
> > approach, we could end up where the first call to Topology.describe()
> > errors out or returns an invalid description, then the second call is
> > successful. So I don't think we'll be able to pursue this approach.
> >
> >
> > John,
> >
> > I initially liked your suggestion, but I also agree with Matthias as to
> why
> > we should not use that approach either.
> >
> > Thanks to all for the comments.
> >
> > Bill
> >
> >
> > On Mon, Jun 11, 2018 at 4:13 PM John Roesler <jo...@confluent.io> wrote:
> >
> > > Thanks Matthias,
> > >
> > > I buy this reasoning.
> > >
> > > -John
> > >
> > > On Mon, Jun 11, 2018 at 12:48 PM, Matthias J. Sax <
> matthias@confluent.io
> > >
> > > wrote:
> > >
> > > > @John: I don't think this is a good idea. `KafkaStreams` executes a
> > > > `Topology` and should be agnostic if the topology was build manually
> or
> > > > via `StreamsBuilder` (at least from my point of view).
> > > >
> > > > -Matthias
> > > >
> > > > On 6/11/18 9:53 AM, Guozhang Wang wrote:
> > > > > Another implementation detail that we can consider: currently the
> > > > > InternalTopologyBuilder#setApplicationId() is used because we do
> not
> > > > have
> > > > > such a mechanism to pass in configs to the topology building
> process.
> > > > Once
> > > > > we add such mechanism we should consider removing this function as
> > > well.
> > > > >
> > > > >
> > > > > Guozhang
> > > > >
> > > > > On Mon, Jun 11, 2018 at 9:51 AM, Guozhang Wang <wangguoz@gmail.com
> >
> > > > wrote:
> > > > >
> > > > >> Hello Bill,
> > > > >>
> > > > >> While working on https://github.com/apache/kafka/pull/5163 I am
> > > > wondering
> > > > >> if we can hide this from the public API, to e.g. add an additional
> > > > function
> > > > >> in InternalTopologyBuilder of InternalStreamsBuilder (since in
> your
> > > > current
> > > > >> working PR we're reusing InternalStreamsBuilder for the logical
> plan
> > > > >> generation) which can then be called inside KafkaStreams
> > constructors?
> > > > >>
> > > > >>
> > > > >> Guozhang
> > > > >>
> > > > >>
> > > > >> On Mon, Jun 11, 2018 at 9:41 AM, John Roesler <jo...@confluent.io>
> > > > wrote:
> > > > >>
> > > > >>> Hi Bill,
> > > > >>>
> > > > >>> Thanks for the KIP.
> > > > >>>
> > > > >>> Just a small thought. This new API will result in calls that look
> > > like
> > > > >>> this:
> > > > >>> new KafkaStreams(builder.build(props), props);
> > > > >>>
> > > > >>> Do you think that's a significant enough eyesore to warrant
> adding
> > a
> > > > new
> > > > >>> KafkaStreams constructor taking a KStreamsBuilder like this:
> > > > >>> new KafkaStreams(builder, props);
> > > > >>>
> > > > >>> such that it would internally call builder.build(props) ?
> > > > >>>
> > > > >>> Thanks,
> > > > >>> -John
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >>> On Fri, Jun 8, 2018 at 7:16 PM, Ted Yu <yu...@gmail.com>
> > wrote:
> > > > >>>
> > > > >>>> Since there're only two values for the optional optimization
> > config
> > > > >>>> introduced by KAFKA-6935, I wonder the overloaded build method
> > (with
> > > > >>>> Properties
> > > > >>>> instance) would make the config unnecessary.
> > > > >>>>
> > > > >>>> nit:
> > > > >>>> * @return @return the {@link Topology} that represents the
> > specified
> > > > >>>> processing logic
> > > > >>>>
> > > > >>>> Double @return above.
> > > > >>>>
> > > > >>>> Cheers
> > > > >>>>
> > > > >>>> On Fri, Jun 8, 2018 at 3:20 PM, Bill Bejeck <bi...@confluent.io>
> > > > wrote:
> > > > >>>>
> > > > >>>>> All,
> > > > >>>>>
> > > > >>>>> I'd like to start the discussion for adding an overloaded
> method
> > to
> > > > >>>>> StreamsBuilder taking a java.util.Properties instance.
> > > > >>>>>
> > > > >>>>> The KIP is located here :
> > > > >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > > >>>>> 312%3A+Add+Overloaded+StreamsBuilder+Build+Method+
> > > > >>>>> to+Accept+java.util.Properties
> > > > >>>>>
> > > > >>>>> I look forward to your comments.
> > > > >>>>>
> > > > >>>>> Thanks,
> > > > >>>>> Bill
> > > > >>>>>
> > > > >>>>
> > > > >>>
> > > > >>
> > > > >>
> > > > >>
> > > > >> --
> > > > >> -- Guozhang
> > > > >>
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> >
>
>
>
> --
> -- Guozhang
>
Re: [DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
Posted by Guozhang Wang <wa...@gmail.com>.
Thanks for the explanation Bill. Makes sense to me.
On Tue, Jun 12, 2018 at 9:21 AM, Bill Bejeck <bb...@gmail.com> wrote:
> > Since there're only two values for the optional optimization config
> > introduced by KAFKA-6935, I wonder the overloaded build method (with
> Properties
> > instance) would make the config unnecessary.
>
> Hi Ted, thanks for commenting. You raise a good point. Buy IMHO, yes we
> still need the config as we want to give users the ability to turn
> optimizations off/on explicitly and we haven't finalized anything
> concerning how we'll pass in the parameters. Additionally, as we release
> new versions, the config will give users the ability to choose to apply all
> of the latest optimizations or stick with the previous version.
>
> Guozhang,
>
> > if we can hide this from the public API, to, e.g. add an additional
> function
> > in InternalTopologyBuilder of InternalStreamsBuilder (since in your
> current
> > working PR we're reusing InternalStreamsBuilder for the logical plan
> > generation) which can then be called inside KafkaStreams constructors?
>
> I like the idea, but as I looked into it, there is an issue concerning the
> fact that users can call Topology.describe() at any point. So with this
> approach, we could end up where the first call to Topology.describe()
> errors out or returns an invalid description, then the second call is
> successful. So I don't think we'll be able to pursue this approach.
>
>
> John,
>
> I initially liked your suggestion, but I also agree with Matthias as to why
> we should not use that approach either.
>
> Thanks to all for the comments.
>
> Bill
>
>
> On Mon, Jun 11, 2018 at 4:13 PM John Roesler <jo...@confluent.io> wrote:
>
> > Thanks Matthias,
> >
> > I buy this reasoning.
> >
> > -John
> >
> > On Mon, Jun 11, 2018 at 12:48 PM, Matthias J. Sax <matthias@confluent.io
> >
> > wrote:
> >
> > > @John: I don't think this is a good idea. `KafkaStreams` executes a
> > > `Topology` and should be agnostic if the topology was build manually or
> > > via `StreamsBuilder` (at least from my point of view).
> > >
> > > -Matthias
> > >
> > > On 6/11/18 9:53 AM, Guozhang Wang wrote:
> > > > Another implementation detail that we can consider: currently the
> > > > InternalTopologyBuilder#setApplicationId() is used because we do not
> > > have
> > > > such a mechanism to pass in configs to the topology building process.
> > > Once
> > > > we add such mechanism we should consider removing this function as
> > well.
> > > >
> > > >
> > > > Guozhang
> > > >
> > > > On Mon, Jun 11, 2018 at 9:51 AM, Guozhang Wang <wa...@gmail.com>
> > > wrote:
> > > >
> > > >> Hello Bill,
> > > >>
> > > >> While working on https://github.com/apache/kafka/pull/5163 I am
> > > wondering
> > > >> if we can hide this from the public API, to e.g. add an additional
> > > function
> > > >> in InternalTopologyBuilder of InternalStreamsBuilder (since in your
> > > current
> > > >> working PR we're reusing InternalStreamsBuilder for the logical plan
> > > >> generation) which can then be called inside KafkaStreams
> constructors?
> > > >>
> > > >>
> > > >> Guozhang
> > > >>
> > > >>
> > > >> On Mon, Jun 11, 2018 at 9:41 AM, John Roesler <jo...@confluent.io>
> > > wrote:
> > > >>
> > > >>> Hi Bill,
> > > >>>
> > > >>> Thanks for the KIP.
> > > >>>
> > > >>> Just a small thought. This new API will result in calls that look
> > like
> > > >>> this:
> > > >>> new KafkaStreams(builder.build(props), props);
> > > >>>
> > > >>> Do you think that's a significant enough eyesore to warrant adding
> a
> > > new
> > > >>> KafkaStreams constructor taking a KStreamsBuilder like this:
> > > >>> new KafkaStreams(builder, props);
> > > >>>
> > > >>> such that it would internally call builder.build(props) ?
> > > >>>
> > > >>> Thanks,
> > > >>> -John
> > > >>>
> > > >>>
> > > >>>
> > > >>> On Fri, Jun 8, 2018 at 7:16 PM, Ted Yu <yu...@gmail.com>
> wrote:
> > > >>>
> > > >>>> Since there're only two values for the optional optimization
> config
> > > >>>> introduced by KAFKA-6935, I wonder the overloaded build method
> (with
> > > >>>> Properties
> > > >>>> instance) would make the config unnecessary.
> > > >>>>
> > > >>>> nit:
> > > >>>> * @return @return the {@link Topology} that represents the
> specified
> > > >>>> processing logic
> > > >>>>
> > > >>>> Double @return above.
> > > >>>>
> > > >>>> Cheers
> > > >>>>
> > > >>>> On Fri, Jun 8, 2018 at 3:20 PM, Bill Bejeck <bi...@confluent.io>
> > > wrote:
> > > >>>>
> > > >>>>> All,
> > > >>>>>
> > > >>>>> I'd like to start the discussion for adding an overloaded method
> to
> > > >>>>> StreamsBuilder taking a java.util.Properties instance.
> > > >>>>>
> > > >>>>> The KIP is located here :
> > > >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > >>>>> 312%3A+Add+Overloaded+StreamsBuilder+Build+Method+
> > > >>>>> to+Accept+java.util.Properties
> > > >>>>>
> > > >>>>> I look forward to your comments.
> > > >>>>>
> > > >>>>> Thanks,
> > > >>>>> Bill
> > > >>>>>
> > > >>>>
> > > >>>
> > > >>
> > > >>
> > > >>
> > > >> --
> > > >> -- Guozhang
> > > >>
> > > >
> > > >
> > > >
> > >
> > >
> >
>
--
-- Guozhang
Re: [DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
Posted by Bill Bejeck <bb...@gmail.com>.
> Since there're only two values for the optional optimization config
> introduced by KAFKA-6935, I wonder the overloaded build method (with
Properties
> instance) would make the config unnecessary.
Hi Ted, thanks for commenting. You raise a good point. Buy IMHO, yes we
still need the config as we want to give users the ability to turn
optimizations off/on explicitly and we haven't finalized anything
concerning how we'll pass in the parameters. Additionally, as we release
new versions, the config will give users the ability to choose to apply all
of the latest optimizations or stick with the previous version.
Guozhang,
> if we can hide this from the public API, to, e.g. add an additional
function
> in InternalTopologyBuilder of InternalStreamsBuilder (since in your
current
> working PR we're reusing InternalStreamsBuilder for the logical plan
> generation) which can then be called inside KafkaStreams constructors?
I like the idea, but as I looked into it, there is an issue concerning the
fact that users can call Topology.describe() at any point. So with this
approach, we could end up where the first call to Topology.describe()
errors out or returns an invalid description, then the second call is
successful. So I don't think we'll be able to pursue this approach.
John,
I initially liked your suggestion, but I also agree with Matthias as to why
we should not use that approach either.
Thanks to all for the comments.
Bill
On Mon, Jun 11, 2018 at 4:13 PM John Roesler <jo...@confluent.io> wrote:
> Thanks Matthias,
>
> I buy this reasoning.
>
> -John
>
> On Mon, Jun 11, 2018 at 12:48 PM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
> > @John: I don't think this is a good idea. `KafkaStreams` executes a
> > `Topology` and should be agnostic if the topology was build manually or
> > via `StreamsBuilder` (at least from my point of view).
> >
> > -Matthias
> >
> > On 6/11/18 9:53 AM, Guozhang Wang wrote:
> > > Another implementation detail that we can consider: currently the
> > > InternalTopologyBuilder#setApplicationId() is used because we do not
> > have
> > > such a mechanism to pass in configs to the topology building process.
> > Once
> > > we add such mechanism we should consider removing this function as
> well.
> > >
> > >
> > > Guozhang
> > >
> > > On Mon, Jun 11, 2018 at 9:51 AM, Guozhang Wang <wa...@gmail.com>
> > wrote:
> > >
> > >> Hello Bill,
> > >>
> > >> While working on https://github.com/apache/kafka/pull/5163 I am
> > wondering
> > >> if we can hide this from the public API, to e.g. add an additional
> > function
> > >> in InternalTopologyBuilder of InternalStreamsBuilder (since in your
> > current
> > >> working PR we're reusing InternalStreamsBuilder for the logical plan
> > >> generation) which can then be called inside KafkaStreams constructors?
> > >>
> > >>
> > >> Guozhang
> > >>
> > >>
> > >> On Mon, Jun 11, 2018 at 9:41 AM, John Roesler <jo...@confluent.io>
> > wrote:
> > >>
> > >>> Hi Bill,
> > >>>
> > >>> Thanks for the KIP.
> > >>>
> > >>> Just a small thought. This new API will result in calls that look
> like
> > >>> this:
> > >>> new KafkaStreams(builder.build(props), props);
> > >>>
> > >>> Do you think that's a significant enough eyesore to warrant adding a
> > new
> > >>> KafkaStreams constructor taking a KStreamsBuilder like this:
> > >>> new KafkaStreams(builder, props);
> > >>>
> > >>> such that it would internally call builder.build(props) ?
> > >>>
> > >>> Thanks,
> > >>> -John
> > >>>
> > >>>
> > >>>
> > >>> On Fri, Jun 8, 2018 at 7:16 PM, Ted Yu <yu...@gmail.com> wrote:
> > >>>
> > >>>> Since there're only two values for the optional optimization config
> > >>>> introduced by KAFKA-6935, I wonder the overloaded build method (with
> > >>>> Properties
> > >>>> instance) would make the config unnecessary.
> > >>>>
> > >>>> nit:
> > >>>> * @return @return the {@link Topology} that represents the specified
> > >>>> processing logic
> > >>>>
> > >>>> Double @return above.
> > >>>>
> > >>>> Cheers
> > >>>>
> > >>>> On Fri, Jun 8, 2018 at 3:20 PM, Bill Bejeck <bi...@confluent.io>
> > wrote:
> > >>>>
> > >>>>> All,
> > >>>>>
> > >>>>> I'd like to start the discussion for adding an overloaded method to
> > >>>>> StreamsBuilder taking a java.util.Properties instance.
> > >>>>>
> > >>>>> The KIP is located here :
> > >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > >>>>> 312%3A+Add+Overloaded+StreamsBuilder+Build+Method+
> > >>>>> to+Accept+java.util.Properties
> > >>>>>
> > >>>>> I look forward to your comments.
> > >>>>>
> > >>>>> Thanks,
> > >>>>> Bill
> > >>>>>
> > >>>>
> > >>>
> > >>
> > >>
> > >>
> > >> --
> > >> -- Guozhang
> > >>
> > >
> > >
> > >
> >
> >
>
Re: [DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
Posted by John Roesler <jo...@confluent.io>.
Thanks Matthias,
I buy this reasoning.
-John
On Mon, Jun 11, 2018 at 12:48 PM, Matthias J. Sax <ma...@confluent.io>
wrote:
> @John: I don't think this is a good idea. `KafkaStreams` executes a
> `Topology` and should be agnostic if the topology was build manually or
> via `StreamsBuilder` (at least from my point of view).
>
> -Matthias
>
> On 6/11/18 9:53 AM, Guozhang Wang wrote:
> > Another implementation detail that we can consider: currently the
> > InternalTopologyBuilder#setApplicationId() is used because we do not
> have
> > such a mechanism to pass in configs to the topology building process.
> Once
> > we add such mechanism we should consider removing this function as well.
> >
> >
> > Guozhang
> >
> > On Mon, Jun 11, 2018 at 9:51 AM, Guozhang Wang <wa...@gmail.com>
> wrote:
> >
> >> Hello Bill,
> >>
> >> While working on https://github.com/apache/kafka/pull/5163 I am
> wondering
> >> if we can hide this from the public API, to e.g. add an additional
> function
> >> in InternalTopologyBuilder of InternalStreamsBuilder (since in your
> current
> >> working PR we're reusing InternalStreamsBuilder for the logical plan
> >> generation) which can then be called inside KafkaStreams constructors?
> >>
> >>
> >> Guozhang
> >>
> >>
> >> On Mon, Jun 11, 2018 at 9:41 AM, John Roesler <jo...@confluent.io>
> wrote:
> >>
> >>> Hi Bill,
> >>>
> >>> Thanks for the KIP.
> >>>
> >>> Just a small thought. This new API will result in calls that look like
> >>> this:
> >>> new KafkaStreams(builder.build(props), props);
> >>>
> >>> Do you think that's a significant enough eyesore to warrant adding a
> new
> >>> KafkaStreams constructor taking a KStreamsBuilder like this:
> >>> new KafkaStreams(builder, props);
> >>>
> >>> such that it would internally call builder.build(props) ?
> >>>
> >>> Thanks,
> >>> -John
> >>>
> >>>
> >>>
> >>> On Fri, Jun 8, 2018 at 7:16 PM, Ted Yu <yu...@gmail.com> wrote:
> >>>
> >>>> Since there're only two values for the optional optimization config
> >>>> introduced by KAFKA-6935, I wonder the overloaded build method (with
> >>>> Properties
> >>>> instance) would make the config unnecessary.
> >>>>
> >>>> nit:
> >>>> * @return @return the {@link Topology} that represents the specified
> >>>> processing logic
> >>>>
> >>>> Double @return above.
> >>>>
> >>>> Cheers
> >>>>
> >>>> On Fri, Jun 8, 2018 at 3:20 PM, Bill Bejeck <bi...@confluent.io>
> wrote:
> >>>>
> >>>>> All,
> >>>>>
> >>>>> I'd like to start the discussion for adding an overloaded method to
> >>>>> StreamsBuilder taking a java.util.Properties instance.
> >>>>>
> >>>>> The KIP is located here :
> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>>>> 312%3A+Add+Overloaded+StreamsBuilder+Build+Method+
> >>>>> to+Accept+java.util.Properties
> >>>>>
> >>>>> I look forward to your comments.
> >>>>>
> >>>>> Thanks,
> >>>>> Bill
> >>>>>
> >>>>
> >>>
> >>
> >>
> >>
> >> --
> >> -- Guozhang
> >>
> >
> >
> >
>
>
Re: [DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
Posted by "Matthias J. Sax" <ma...@confluent.io>.
@John: I don't think this is a good idea. `KafkaStreams` executes a
`Topology` and should be agnostic if the topology was build manually or
via `StreamsBuilder` (at least from my point of view).
-Matthias
On 6/11/18 9:53 AM, Guozhang Wang wrote:
> Another implementation detail that we can consider: currently the
> InternalTopologyBuilder#setApplicationId() is used because we do not have
> such a mechanism to pass in configs to the topology building process. Once
> we add such mechanism we should consider removing this function as well.
>
>
> Guozhang
>
> On Mon, Jun 11, 2018 at 9:51 AM, Guozhang Wang <wa...@gmail.com> wrote:
>
>> Hello Bill,
>>
>> While working on https://github.com/apache/kafka/pull/5163 I am wondering
>> if we can hide this from the public API, to e.g. add an additional function
>> in InternalTopologyBuilder of InternalStreamsBuilder (since in your current
>> working PR we're reusing InternalStreamsBuilder for the logical plan
>> generation) which can then be called inside KafkaStreams constructors?
>>
>>
>> Guozhang
>>
>>
>> On Mon, Jun 11, 2018 at 9:41 AM, John Roesler <jo...@confluent.io> wrote:
>>
>>> Hi Bill,
>>>
>>> Thanks for the KIP.
>>>
>>> Just a small thought. This new API will result in calls that look like
>>> this:
>>> new KafkaStreams(builder.build(props), props);
>>>
>>> Do you think that's a significant enough eyesore to warrant adding a new
>>> KafkaStreams constructor taking a KStreamsBuilder like this:
>>> new KafkaStreams(builder, props);
>>>
>>> such that it would internally call builder.build(props) ?
>>>
>>> Thanks,
>>> -John
>>>
>>>
>>>
>>> On Fri, Jun 8, 2018 at 7:16 PM, Ted Yu <yu...@gmail.com> wrote:
>>>
>>>> Since there're only two values for the optional optimization config
>>>> introduced by KAFKA-6935, I wonder the overloaded build method (with
>>>> Properties
>>>> instance) would make the config unnecessary.
>>>>
>>>> nit:
>>>> * @return @return the {@link Topology} that represents the specified
>>>> processing logic
>>>>
>>>> Double @return above.
>>>>
>>>> Cheers
>>>>
>>>> On Fri, Jun 8, 2018 at 3:20 PM, Bill Bejeck <bi...@confluent.io> wrote:
>>>>
>>>>> All,
>>>>>
>>>>> I'd like to start the discussion for adding an overloaded method to
>>>>> StreamsBuilder taking a java.util.Properties instance.
>>>>>
>>>>> The KIP is located here :
>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>>>> 312%3A+Add+Overloaded+StreamsBuilder+Build+Method+
>>>>> to+Accept+java.util.Properties
>>>>>
>>>>> I look forward to your comments.
>>>>>
>>>>> Thanks,
>>>>> Bill
>>>>>
>>>>
>>>
>>
>>
>>
>> --
>> -- Guozhang
>>
>
>
>
Re: [DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
Posted by Guozhang Wang <wa...@gmail.com>.
Another implementation detail that we can consider: currently the
InternalTopologyBuilder#setApplicationId() is used because we do not have
such a mechanism to pass in configs to the topology building process. Once
we add such mechanism we should consider removing this function as well.
Guozhang
On Mon, Jun 11, 2018 at 9:51 AM, Guozhang Wang <wa...@gmail.com> wrote:
> Hello Bill,
>
> While working on https://github.com/apache/kafka/pull/5163 I am wondering
> if we can hide this from the public API, to e.g. add an additional function
> in InternalTopologyBuilder of InternalStreamsBuilder (since in your current
> working PR we're reusing InternalStreamsBuilder for the logical plan
> generation) which can then be called inside KafkaStreams constructors?
>
>
> Guozhang
>
>
> On Mon, Jun 11, 2018 at 9:41 AM, John Roesler <jo...@confluent.io> wrote:
>
>> Hi Bill,
>>
>> Thanks for the KIP.
>>
>> Just a small thought. This new API will result in calls that look like
>> this:
>> new KafkaStreams(builder.build(props), props);
>>
>> Do you think that's a significant enough eyesore to warrant adding a new
>> KafkaStreams constructor taking a KStreamsBuilder like this:
>> new KafkaStreams(builder, props);
>>
>> such that it would internally call builder.build(props) ?
>>
>> Thanks,
>> -John
>>
>>
>>
>> On Fri, Jun 8, 2018 at 7:16 PM, Ted Yu <yu...@gmail.com> wrote:
>>
>> > Since there're only two values for the optional optimization config
>> > introduced by KAFKA-6935, I wonder the overloaded build method (with
>> > Properties
>> > instance) would make the config unnecessary.
>> >
>> > nit:
>> > * @return @return the {@link Topology} that represents the specified
>> > processing logic
>> >
>> > Double @return above.
>> >
>> > Cheers
>> >
>> > On Fri, Jun 8, 2018 at 3:20 PM, Bill Bejeck <bi...@confluent.io> wrote:
>> >
>> > > All,
>> > >
>> > > I'd like to start the discussion for adding an overloaded method to
>> > > StreamsBuilder taking a java.util.Properties instance.
>> > >
>> > > The KIP is located here :
>> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > 312%3A+Add+Overloaded+StreamsBuilder+Build+Method+
>> > > to+Accept+java.util.Properties
>> > >
>> > > I look forward to your comments.
>> > >
>> > > Thanks,
>> > > Bill
>> > >
>> >
>>
>
>
>
> --
> -- Guozhang
>
--
-- Guozhang
Re: [DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
Posted by Guozhang Wang <wa...@gmail.com>.
Hello Bill,
While working on https://github.com/apache/kafka/pull/5163 I am wondering
if we can hide this from the public API, to e.g. add an additional function
in InternalTopologyBuilder of InternalStreamsBuilder (since in your current
working PR we're reusing InternalStreamsBuilder for the logical plan
generation) which can then be called inside KafkaStreams constructors?
Guozhang
On Mon, Jun 11, 2018 at 9:41 AM, John Roesler <jo...@confluent.io> wrote:
> Hi Bill,
>
> Thanks for the KIP.
>
> Just a small thought. This new API will result in calls that look like
> this:
> new KafkaStreams(builder.build(props), props);
>
> Do you think that's a significant enough eyesore to warrant adding a new
> KafkaStreams constructor taking a KStreamsBuilder like this:
> new KafkaStreams(builder, props);
>
> such that it would internally call builder.build(props) ?
>
> Thanks,
> -John
>
>
>
> On Fri, Jun 8, 2018 at 7:16 PM, Ted Yu <yu...@gmail.com> wrote:
>
> > Since there're only two values for the optional optimization config
> > introduced by KAFKA-6935, I wonder the overloaded build method (with
> > Properties
> > instance) would make the config unnecessary.
> >
> > nit:
> > * @return @return the {@link Topology} that represents the specified
> > processing logic
> >
> > Double @return above.
> >
> > Cheers
> >
> > On Fri, Jun 8, 2018 at 3:20 PM, Bill Bejeck <bi...@confluent.io> wrote:
> >
> > > All,
> > >
> > > I'd like to start the discussion for adding an overloaded method to
> > > StreamsBuilder taking a java.util.Properties instance.
> > >
> > > The KIP is located here :
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 312%3A+Add+Overloaded+StreamsBuilder+Build+Method+
> > > to+Accept+java.util.Properties
> > >
> > > I look forward to your comments.
> > >
> > > Thanks,
> > > Bill
> > >
> >
>
--
-- Guozhang
Re: [DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
Posted by John Roesler <jo...@confluent.io>.
Hi Bill,
Thanks for the KIP.
Just a small thought. This new API will result in calls that look like this:
new KafkaStreams(builder.build(props), props);
Do you think that's a significant enough eyesore to warrant adding a new
KafkaStreams constructor taking a KStreamsBuilder like this:
new KafkaStreams(builder, props);
such that it would internally call builder.build(props) ?
Thanks,
-John
On Fri, Jun 8, 2018 at 7:16 PM, Ted Yu <yu...@gmail.com> wrote:
> Since there're only two values for the optional optimization config
> introduced by KAFKA-6935, I wonder the overloaded build method (with
> Properties
> instance) would make the config unnecessary.
>
> nit:
> * @return @return the {@link Topology} that represents the specified
> processing logic
>
> Double @return above.
>
> Cheers
>
> On Fri, Jun 8, 2018 at 3:20 PM, Bill Bejeck <bi...@confluent.io> wrote:
>
> > All,
> >
> > I'd like to start the discussion for adding an overloaded method to
> > StreamsBuilder taking a java.util.Properties instance.
> >
> > The KIP is located here :
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 312%3A+Add+Overloaded+StreamsBuilder+Build+Method+
> > to+Accept+java.util.Properties
> >
> > I look forward to your comments.
> >
> > Thanks,
> > Bill
> >
>
Re: [DISCUSS] KIP-312: Add Overloaded StreamsBuilder Build Method to
Accept java.util.Properties
Posted by Ted Yu <yu...@gmail.com>.
Since there're only two values for the optional optimization config
introduced by KAFKA-6935, I wonder the overloaded build method (with Properties
instance) would make the config unnecessary.
nit:
* @return @return the {@link Topology} that represents the specified
processing logic
Double @return above.
Cheers
On Fri, Jun 8, 2018 at 3:20 PM, Bill Bejeck <bi...@confluent.io> wrote:
> All,
>
> I'd like to start the discussion for adding an overloaded method to
> StreamsBuilder taking a java.util.Properties instance.
>
> The KIP is located here :
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 312%3A+Add+Overloaded+StreamsBuilder+Build+Method+
> to+Accept+java.util.Properties
>
> I look forward to your comments.
>
> Thanks,
> Bill
>