+1 (binding), thank you Nikolay! Guozhang
On Thu, Sep 13, 2018 at 9:39 AM, Matthias J. Sax <matth...@confluent.io> wrote: > Thanks for the KIP. > > +1 (binding) > > > -Matthias > > On 9/5/18 8:52 AM, John Roesler wrote: > > I'm a +1 (non-binding) > > > > On Mon, Sep 3, 2018 at 8:33 AM Nikolay Izhikov <nizhi...@apache.org> > wrote: > > > >> Dear commiters. > >> > >> Please, vote on a KIP. > >> > >> В Пт, 31/08/2018 в 12:05 -0500, John Roesler пишет: > >>> Hi Nikolay, > >>> > >>> You can start a PR any time, but we cannot per it (and probably won't > do > >>> serious reviews) until after the KIP is voted and approved. > >>> > >>> Sometimes people start a PR during discussion just to help provide more > >>> context, but it's not required (and can also be distracting because the > >> KIP > >>> discussion should avoid implementation details). > >>> > >>> Let's wait one more day for any other comments and plan to start the > vote > >>> on Monday if there are no other debates. > >>> > >>> Once you start the vote, you have to leave it up for at least 72 hours, > >> and > >>> it requires 3 binding votes to pass. Only Kafka Committers have binding > >>> votes (https://kafka.apache.org/committers). > >>> > >>> Thanks, > >>> -John > >>> > >>> On Fri, Aug 31, 2018 at 11:09 AM Bill Bejeck <bbej...@gmail.com> > wrote: > >>> > >>>> Hi Nickolay, > >>>> > >>>> Thanks for the clarification. > >>>> > >>>> -Bill > >>>> > >>>> On Fri, Aug 31, 2018 at 11:59 AM Nikolay Izhikov <nizhi...@apache.org > > > >>>> wrote: > >>>> > >>>>> Hello, John. > >>>>> > >>>>> This is my first KIP, so, please, help me with kafka development > >> process. > >>>>> > >>>>> Should I start to work on PR now? Or should I wait for a "+1" from > >>>>> commiters? > >>>>> > >>>>> В Пт, 31/08/2018 в 10:33 -0500, John Roesler пишет: > >>>>>> I see. I guess that once we are in the PR-reviewing phase, we'll > >> be in > >>>> > >>>> a > >>>>>> better position to see what else can/should be done, and we can > >> talk > >>>>> > >>>>> about > >>>>>> follow-on work at that time. > >>>>>> > >>>>>> Thanks for the clarification, > >>>>>> -John > >>>>>> > >>>>>> On Fri, Aug 31, 2018 at 1:19 AM Nikolay Izhikov < > >> nizhi...@apache.org> > >>>>> > >>>>> wrote: > >>>>>> > >>>>>>> Hello, Bill > >>>>>>> > >>>>>>>> In the "Proposed Changes" section, there is "Try to reduce the > >>>>>>> > >>>>>>> visibility of methods in next tickets" does that mean eventual > >>>>> > >>>>> deprecation > >>>>>>> and removal? > >>>>>>> > >>>>>>> 1. Some methods will become deprecated. I think they will be > >> removed > >>>> > >>>> in > >>>>>>> the future. > >>>>>>> You can find list of deprecated methods in KIP. > >>>>>>> > >>>>>>> 2. Some internal methods can't be deprecated or hid from the > >> user for > >>>>> > >>>>> now. > >>>>>>> I was trying to say that we should research possibility to reduce > >>>>>>> visibility of *internal* methods that are *public* now. > >>>>>>> That kind of changes is out of the scope of current KIP, so we > >> have > >>>> > >>>> to > >>>>> do > >>>>>>> it in the next tickets. > >>>>>>> > >>>>>>> I don't expect that internal methods will be removed. > >>>>>>> > >>>>>>> В Чт, 30/08/2018 в 18:59 -0400, Bill Bejeck пишет: > >>>>>>>> Sorry for chiming in late, there was a lot of detail to catch > >> up > >>>> > >>>> on. > >>>>>>>> > >>>>>>>> Overall I'm +1 in the KIP. But I do have one question about > >> the > >>>> > >>>> KIP > >>>>> in > >>>>>>>> regards to Matthias's comments about defining dual use. > >>>>>>>> > >>>>>>>> In the "Proposed Changes" section, there is "Try to reduce the > >>>>> > >>>>> visibility > >>>>>>>> of methods in next tickets" does that mean eventual > >> deprecation and > >>>>>>> > >>>>>>> removal? > >>>>>>>> I thought we were aiming to keep the dual use methods? Or does > >> that > >>>>> > >>>>> imply > >>>>>>>> we'll strive for more clear delineation between DSL and > >> internal > >>>> > >>>> use? > >>>>>>>> > >>>>>>>> Thanks, > >>>>>>>> Bill > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On Thu, Aug 30, 2018 at 5:59 PM Nikolay Izhikov < > >>>> > >>>> nizhi...@apache.org > >>>>>> > >>>>>>> > >>>>>>> wrote: > >>>>>>>> > >>>>>>>>> John, thank you. > >>>>>>>>> > >>>>>>>>> I've updated KIP. > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> Dear commiters, please take a look and share your opinion. > >>>>>>>>> > >>>>>>>>> В Чт, 30/08/2018 в 14:58 -0500, John Roesler пишет: > >>>>>>>>>> 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 > >>>> > > > > -- -- Guozhang
