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
