Thanks for the update, Matthias. +1 to the points 1,2,3,4 you mentioned.
Naming is always a tricky subject, but renaming KStreamBuilder to StreamsTopologyBuilder looks ok to me (I would have had a slight preference towards DslTopologyBuilder, but hey.) The most important aspect is, IMHO, what you also pointed out: to make it clear that the current KStreamBuilder actually builds a topology (though currently the latter is actually called `TopologyBuilder` currently), and not a `KStream`. On Wed, Mar 8, 2017 at 11:43 PM, Matthias J. Sax <matth...@confluent.io> wrote: > Hi, > > sorry for not replying earlier and thanks for all your feedback. After > some more discussions I updated the KIP. The new proposal puts some > other design considerations into account, that I want to highlight > shortly. Those considerations, automatically resolve the concerns raised. > > First some answers: > > > The PAPI processors I use in my KStreams app are all functioning on > KTable > > internals. I wouldn't be able to convert them to process()/transform(). > > > > What's the harm in permitting both APIs to be used in the same > application? > > It's not about "harm" but about design. We want to switch from a > "inheritance" to a "composition" pattern. > > About the interface idea: using a shared interface would not help to get > a composition pattern > > > Next I want to give the design considerations leading to the updated KIP: > > 1) Using KStreamBuilder in the constructor of KafkaStreams is unnatural. > KafkaStreams client executes a `Topology` and this execution should be > independent of the way the topology is "put together", ie, low-level API > or DSL. > > 2) Thus, we don't want to have any changes to KafkaStreams class. > > 3) Thus, KStreamBuilder needs to have a method `build()` that returns a > `Topology` that can be passed into KafakStreams. > > 4) Because `KStreamBuilder` should build a `Topology` I suggest to > rename the new class to `StreamsTopologyBuilder` (the name > TopologyBuilder would actually be more natural, but would be easily > confused with old low-level API TopologyBuilder). > > Thus, PAPI and DSL can be mixed-and-matched with full power, as > StreamsTopologyBuilder return the created Topology via #build(). > > I also removed `final` for both builder classes. > > > > With regard to the larger scope of the overal API redesign, I also want > to point to a summary of API issues: > https://cwiki.apache.org/confluence/display/KAFKA/ > Kafka+Streams+Discussions > > Thus, this KIP is only one building block of a larger improvement > effort, and we hope to get as much as possible done for 0.11. If you > have any API improvement ideas, please share them so we can come up with > an holistic sound design (instead of uncoordinated local improvements > that might diverge) > > > > Looking forward to your feedback on this KIP and the other API issues. > > > > -Matthias > > > > > On 2/15/17 7:36 PM, Mathieu Fenniak wrote: > > On Wed, Feb 15, 2017 at 5:04 PM, Matthias J. Sax <matth...@confluent.io> > > wrote: > > > >> - We also removed method #topologyBuilder() from KStreamBuilder because > >> we think #transform() should provide all functionality you need to > >> mix-an-match Processor API and DSL. If there is any further concern > >> about this, please let us know. > >> > > > > Hi Matthias, > > > > Yes, I'm sorry I didn't respond sooner, but I still have a lot of > concerns > > about this. You're correct to point out that transform() can be used for > > some of the output situations I pointed out; albeit it seems somewhat > > awkward to do so in a "transform" method; what do you do with the retval? > > > > The PAPI processors I use in my KStreams app are all functioning on > KTable > > internals. I wouldn't be able to convert them to process()/transform(). > > > > What's the harm in permitting both APIs to be used in the same > application? > > > > Mathieu > > > >
