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 > > >
