Thanks Matthias. 1) Yes good suggestion will update. 2) As it is consistent with Aggregator and a developer may want to use the key. So why not? 3) Thanks. I'll update the KIP
Cheers, Damian On Tue, 29 Nov 2016 at 23:47 Matthias J. Sax <matth...@confluent.io> wrote: > Very nice KIP! > > > Couple of comments: > > 1) Current window API is as follows: > > > > JoinWindows.of(timeDifference).before(timeDifference).after(timeDifference) > > TimeWindows.of(size).advanceBy(interval) > > UnlimitedWindow.of().startOn(start) > > To align with this scheme, I think it would be better to use the > following API for SessionWindows > > > SessionWindows.with(inactivityGap) > > > 2) I am wondering, why SessionMerger does need the key? > > 3) You KIP API for SessionWindows and you PR does not align. There are > some getters in you code that are not part of the KIP (not sure how > important this is) > > > -Matthias > > > > On 11/24/16 7:59 AM, Damian Guy wrote: > > Hi all, > > > > I would like to start the discussion on KIP-94: > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-94+Session+Windows > > > > Thanks, > > Damian > > > >
