> 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 <j...@confluent.io> wrote: > Thanks Matthias, > > I buy this reasoning. > > -John > > On Mon, Jun 11, 2018 at 12:48 PM, Matthias J. Sax <matth...@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 <wangg...@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 <j...@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 <yuzhih...@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 <b...@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 > > >> > > > > > > > > > > > > > >
