Oh! I missed one minor thing: UnlimitedWindows doesn't need to set grace (it currently does not either).
Otherwise, it looks good to me! Thanks so much, -John On Thu, Aug 30, 2018 at 5:30 AM Nikolay Izhikov <nizhi...@apache.org> wrote: > Hello, John. > > I've updated KIP according on your comments. > Please, take a look. > > Are we ready to vot now? > > В Ср, 29/08/2018 в 14:51 -0500, John Roesler пишет: > > Hey Nikolay, sorry for the silence. I'm taking another look at the KIP > > before voting... > > > > > > 1. I think the Window constructor should actually be protected. I > don't > > know if we need a constructor that takes Instant, but if we do add > one, it > > should definitely be protected. > > 2. `long JoinWindows#size()` is overridden from `long Windows#size()`, > > and should not be deprecated. Also, I don't think we need a `Duration > > JoinWindows#windowSize()`. > > 3. Likewise, `JoinWindows#windowsFor()` is overridden from > > `Windows#windowsFor()` and should also not be deprecated, and we also > don't > > need a `Map<Instant, Window> windowsForTime(final Instant timestamp)` > > version. > > 4. TimeWindowedDeserializer is a bit of a puzzle for me. It actually > > looks like it's incorrectly implemented! I'm not sure if we want/need > to > > update any of its methods or constructors. > > 5. TimeWindows: see my feedback on JoinWindows > > 6. UnlimitedWindows: see my feedback on JoinWindows > > 7. ReadOnlyWindowStore: the existing `long` methods should be > > deprecated. (we should add `WindowStoreIterator<V> fetch(K key, long > > timeFrom, long timeTo)` to WindowStore) > > 8. SessionBytesStoreSupplier: Both of those methods are "internal use > > methods", so we should just leave them alone and not add new ones. > > 9. SessionStore: I don't think these are "external use" methods (only > > ReadOnlySessionStore is used in IQ) maybe we should just leave them > alone? > > 10. Stores: I think we can just deprecate without replacement the > method > > that takes `segmentInterval`. > > 11. WindowBytesStoreSupplier: I think this interface is also "internal > > use" and can be left alone > > > > Thank you for the very clear KIP that makes this discussion possible. In > > general, to justify some of those comments, it's easier to add missing > > methods later on than to remove them, so I'm erring on the side of only > > adding new variants when they show up in DSL code, not worrying about the > > lower-level APIs. > > > > What do you think about this? > > -John > > > > On Wed, Aug 29, 2018 at 11:14 AM Nikolay Izhikov <nizhi...@apache.org> > > wrote: > > > > > Hello, All. > > > > > > Calling a vote on KIP-358 [1] > > > > > > [1] > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-358%3A+Migrate+Streams+API+to+Duration+instead+of+long+ms+times
