About Windows: We got the request to add TimeWindows etc to public API anyway. I guess it's helpful for people adding their own window aggregates using low-level API.
But as mentioned already, it was just a thought, and I am also not 100% sure if we would gain much by changing the return type. Thus, if we don't see any advantage, we can just keep it as is. Same for stores: if you think the store API is stable, we don't need @Envolving -- we just got couple of KIPs about store API lately, so I wanted to double check only :) -Matthias On 5/16/17 3:49 PM, Guozhang Wang wrote: > Hello Matthias, > > Reply inlined. > > On Sat, May 13, 2017 at 1:40 PM, Matthias J. Sax <matth...@confluent.io> > wrote: > >> +1 for the overall proposal. >> >> Some comments: >> >> Currently, `Windowed#window()` returns type `Window` and I am wondering >> if we should improve type safety here and return the actual window type >> (ie, TimeWindow, SessionWindow, JoinWindow) etc. >> >> If this would be a useful improvement (frankly speaking, I am not 100% >> sure if we need this), we should mark `Windowed` as @evolving, too. >> >> > Hmm, I'm not sure, since TimeWindow / etc are internal classes. If we want > to expose them I need to expose all these internal classes as part of the > public APIs. But these internal classes do not expose any more functions > for users to call either. > > >> >> I am also no 100% sure about package `state` (or did you mean "top level >> package "o.a.k.streams" as you mention "StreamsMetric" in the same >> paragraph) -- are we sure it's stable enough to remove the annotation? >> Or should we use @evolving here, too? >> >> > Which class are you mostly concerning about? > > >> With KIP-120 in the pipeline, we should also add @evolving to >> KafkaStreams IMHO. >> >> > Yeah I forgot to mention that for the top-level classes should be > @evolving, including StreamsMetrics (I was wrong in the previous email, it > is not in `o.a.k.streams.state`) > > >> >> -Matthias >> >> On 5/11/17 3:48 AM, Eno Thereska wrote: >>> Sounds reasonable. >>> >>> Thanks, >>> Eno >>>> On May 11, 2017, at 7:39 AM, Ismael Juma <ism...@juma.me.uk> wrote: >>>> >>>> Thanks for the proposal Guozhang. This sounds good to me. >>>> >>>> Ismael >>>> >>>> On Thu, May 11, 2017 at 6:02 AM, Guozhang Wang <wangg...@gmail.com> >> wrote: >>>> >>>>> Hello folks, >>>>> >>>>> As we are approaching the feature freeze deadline of 0.11.0.0, one >> thing I >>>>> realized is that currently the Streams public APIs are still marked as >>>>> "Unstable", which is to indicate that the API itself does not provide >>>>> guarantees about backward compatibility across releases. On the other >> hand, >>>>> since Streams have now been widely adopted in production use cases by >> many >>>>> organizations, we are in fact evolving its APIs in a much stricter >> manner >>>>> than "Unstable" allows us: for all the current Streams related KIP >>>>> proposals under discussions right now [1], people have been working >> hard to >>>>> make sure none of them are going to break backward compatibility in the >>>>> coming releases. So I think it would be a good timing to change the >> Streams >>>>> API annotations. >>>>> >>>>> My proposal would be the following: >>>>> >>>>> 1. For "o.a.k.streams.errors" and "o.a.k.streams.state" packages: >> remove >>>>> the annotations except `StreamsMetrics`. >>>>> >>>>> 2. For "o.a.k.streams.kstream": remove the annotations except >> "KStream", >>>>> "KTable", "GroupedKStream", "GroupedKTable", "GlobalKTable" and >>>>> "KStreamBuilder". >>>>> >>>>> 3. For all the other public classes, including >> "o.a.k.streams.processor" >>>>> and the above mentioned classes, change the annotation to "Evolving", >> which >>>>> means "we might break compatibility at minor releases (i.e. 0.12.x, >> 0.13.x, >>>>> 1.0.x etc) only". >>>>> >>>>> >>>>> The ultimate goal is to make sure we won't break anything going >> forward, >>>>> hence in the future we should remove all the annotations to make that >>>>> clear. The above changes in 0.11.0.0 is to give us some "buffer time" >> in >>>>> case there are some major API change proposals after the release. >>>>> >>>>> Would love to hear your thoughts. >>>>> >>>>> >>>>> [1] >>>>> >>>>> KIP-95: Incremental Batch Processing for Kafka Streams >>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>> 95%3A+Incremental+Batch+Processing+for+Kafka+Streams> >>>>> >>>>> KIP-120: Cleanup Kafka Streams builder API >>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>> 120%3A+Cleanup+Kafka+Streams+builder+API> >>>>> >>>>> KIP-123: Allow per stream/table timestamp extractor >>>>> <https://cwiki.apache.org/confluence/pages/viewpage. >> action?pageId=68714788 >>>>>> >>>>> >>>>> KIP 130: Expose states of active tasks to KafkaStreams public API >>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP+ >>>>> 130%3A+Expose+states+of+active+tasks+to+KafkaStreams+public+API> >>>>> >>>>> KIP-132: Augment KStream.print to allow extra parameters in the printed >>>>> string >>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>> 132+-+Augment+KStream.print+to+allow+extra+parameters+in+ >>>>> the+printed+string> >>>>> >>>>> KIP-138: Change punctuate semantics >>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>> 138%3A+Change+punctuate+semantics> >>>>> >>>>> KIP-147: Add missing type parameters to StateStoreSupplier factories >> and >>>>> KGroupedStream/Table methods >>>>> <https://cwiki.apache.org/confluence/pages/viewpage. >> action?pageId=69408481 >>>>>> >>>>> >>>>> KIP-149: Enabling key access in ValueTransformer, ValueMapper, and >>>>> ValueJoiner >>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>> 149%3A+Enabling+key+access+in+ValueTransformer%2C+ValueMapper%2C+and+ >>>>> ValueJoiner#KIP-149:EnablingkeyaccessinValueTransformer,ValueMapper, >>>>> andValueJoiner-RejectedAlternatives> >>>>> >>>>> KIP-150 - Kafka-Streams Cogroup >>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP- >>>>> 150+-+Kafka-Streams+Cogroup> >>>>> >>>>> KIP 155 - Add range scan for windowed state stores >>>>> <https://cwiki.apache.org/confluence/display/KAFKA/KIP+ >>>>> 155+-+Add+range+scan+for+windowed+state+stores> >>>>> >>>>> KIP 156 Add option "dry run" to Streams application reset tool >>>>> <https://cwiki.apache.org/confluence/pages/viewpage. >> action?pageId=69410150 >>>>>> >>>>> >>>>> >>>>> -- >>>>> -- Guozhang >>>>> >>> >> >> > >
signature.asc
Description: OpenPGP digital signature
