I think changing it to `toKStream` would make it absolutely clear what we are converting it to.
I'd say we should probably change the KStreamBuilder methods (but not in this KIP). Thanks Eno > On 17 Jan 2017, at 13:59, Michael Noll <mich...@confluent.io> wrote: > >> Rename toStream() to toKStream() for consistency. > > Not sure whether that is really required. We also use > `KStreamBuilder#stream()` and `KStreamBuilder#table()`, for example, and > don't care about the "K" prefix. > > > > On Tue, Jan 17, 2017 at 10:55 AM, Eno Thereska <eno.there...@gmail.com> > wrote: > >> Thanks Damian, answers inline: >> >>> On 16 Jan 2017, at 17:17, Damian Guy <damian....@gmail.com> wrote: >>> >>> Hi Eno, >>> >>> Thanks for the KIP. Some comments: >>> >>> 1. I'd probably rename materialized to materialize. >> >> Ok. >> >>> 2. I don't think the addition of the new Log compaction mechanism is >>> necessary for this KIP, i.e, the KIP is useful without it. Maybe that >>> should be a different KIP? >> >> Agreed, already removed. Will do a separate KIP for that. >> >> >>> 3. What will happen when you call materialize on KTable that is already >>> materialized? Will it create another StateStore (providing the name is >>> different), throw an Exception? >> >> Currently an exception is thrown, but see below. >> >> >>> 4. Have you considered overloading the existing KTable operations to >> add >>> a state store name? So if a state store name is provided, then >> materialize >>> a state store? This would be my preferred approach as i don't think >>> materialize is always a valid operation. >> >> Ok I can see your point. This will increase the KIP size since I'll need >> to enumerate all overloaded methods, but it's not a problem. >> >>> 5. The materialize method will need ta value Serde as some operations, >>> i.e., mapValues, join etc can change the value types >>> 6. https://issues.apache.org/jira/browse/KAFKA-4609 - might mean that >> we >>> always need to materialize the StateStore for KTable-KTable joins. If >> that >>> is the case, then the KTable Join operators will also need Serde >>> information. >> >> I'll update the KIP with the serdes. >> >> Thanks >> Eno >> >> >>> >>> Cheers, >>> Damian >>> >>> >>> On Mon, 16 Jan 2017 at 16:44 Eno Thereska <eno.there...@gmail.com> >> wrote: >>> >>>> Hello, >>>> >>>> We created "KIP-114: KTable materialization and improved semantics" to >>>> solidify the KTable semantics in Kafka Streams: >>>> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- >> 114%3A+KTable+materialization+and+improved+semantics >>>> < >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- >> 114:+KTable+materialization+and+improved+semantics >>>>> >>>> >>>> Your feedback is appreciated. >>>> Thanks >>>> Eno >> >>
