+1 from myself as well. I'm closing this thread with the tally below:
binding +1: 3 (Damian, Matthias, myself) non-binding +1: 3 (Ted, Bill, John) Thanks for everyone's votes! Guozhang On Fri, Jun 29, 2018 at 1:15 PM, Matthias J. Sax <matth...@confluent.io> wrote: > Thx. That makes sense. Just want to make sure the KIP clearly covers it. > > > +1 (binding) > > > I am only +1 because I think that there are no users customizing a > session store supplier. This KIP goes into 2.1 what is a minor release > that should not contain any breaking changes. We need to be careful with > changes like this and should try to avoid them. > > > -Matthias > > > On 6/29/18 1:02 PM, Guozhang Wang wrote: > > Thanks Matthias, thanks for the comments! Ack on all of them and have > > updated the wiki: > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > 330:+Add+retentionPeriod+in+SessionBytesStoreSupplier > > > > > > Regarding the impact of this KIP, here's what I'm thinking: > > > > 1. For users of DSL, with customized session store, they need to pass in > > that SessionBytesStoreSupplier into Materialized, and hence need to > always > > customize that interface and hence be impacted. > > 2. For users of PAPI, they will be addStore with a `StoreBuilder`. Note > > that users can either customize the SessionBytesStoreSupplier and pass > into > > `Stores#persistentSessionStore`, or they can go directly instantiate the > > `StoreBuilder` interface. For the latter case, they are not impacted. > > > > Does that make sense to you? > > > > Guozhang > > > > > > On Fri, Jun 29, 2018 at 10:44 AM Matthias J. Sax <matth...@confluent.io> > > wrote: > > > >> The KIP says: > >> > >>> In WindowBytesStoreSupplier, we will add: > >> > >> Should it be `SessionBytesStoreSupplier` ? > >> > >> > >> What do you mean by > >> > >>> Users customizing the SessionBytesStoreSupplier should not implement > >> this function. > >> > >> From my understanding, this is a breaking change for all users > >> implementing a custom `session window store`. I would expect that there > >> are very few users but the KIP should state this clearly as a breaking > >> API change. > >> > >> > >> Nit: the JIRA link seems to be wrong. > >> > >> > >> -Matthias > >> > >> On 6/28/18 10:12 AM, John Roesler wrote: > >>> +1 > >>> > >>> On Thu, Jun 28, 2018 at 4:39 AM Damian Guy <damian....@gmail.com> > wrote: > >>> > >>>> +1 > >>>> > >>>> On Thu, 28 Jun 2018 at 02:16 Ted Yu <yuzhih...@gmail.com> wrote: > >>>> > >>>>> +1 > >>>>> > >>>>> On Wed, Jun 27, 2018 at 4:40 PM, Bill Bejeck <bbej...@gmail.com> > >> wrote: > >>>>> > >>>>>> +1 > >>>>>> > >>>>>> -Bill > >>>>>> > >>>>>> On Wed, Jun 27, 2018 at 7:39 PM Guozhang Wang <wangg...@gmail.com> > >>>>> wrote: > >>>>>> > >>>>>>> Hello folks, > >>>>>>> > >>>>>>> I'd like to start a voting thread on KIP-330. I've intentionally > >>>>> skipped > >>>>>>> the discuss phase since it is a pretty straight-forward public API > >>>>> change > >>>>>>> and should actually be added since day one. The bug fix of > KAFKA-7071 > >>>>>>> helped us to discover this overlook. > >>>>>>> > >>>>>>> > >>>>>>> -- > >>>>>>> -- Guozhang > >>>>>>> > >>>>>> > >>>>> > >>>> > >>> > >> > >> > > > > -- -- Guozhang
