Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2021-01-21 Thread Leah Thomas
Thanks everyone, with that I'll go ahead and close this KIP with 4 binding votes (Sophie, Guozhang, John, Matthias). Thanks for the lively discussion! Leah On Wed, Jan 20, 2021 at 6:00 PM Matthias J. Sax wrote: > Thanks for reviving this KIP, Leah. > > I agree that we should not extend the

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2021-01-20 Thread Matthias J. Sax
Thanks for reviving this KIP, Leah. I agree that we should not extend the scope of this KIP to potentially deprecate/rename the `default.windowed.[key|value].serde.inner` configs. @Sophie: if you feel strong about it, let's do a separate KIP. +1 (binding) -Matthias On 1/19/21 3:00 PM, John

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2021-01-19 Thread John Roesler
Hi all, I've just caught up on the thread, and FWIW, I'm still +1. Thanks, -John On Mon, 2021-01-18 at 21:53 -0800, Guozhang Wang wrote: > Read the above updates and the KIP's scope. Makes sense to me. +1 still > counts :) > > On Wed, Jan 13, 2021 at 2:04 PM Sophie Blee-Goldman > wrote: > >

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2021-01-18 Thread Guozhang Wang
Read the above updates and the KIP's scope. Makes sense to me. +1 still counts :) On Wed, Jan 13, 2021 at 2:04 PM Sophie Blee-Goldman wrote: > That sounds good to me. Thanks for reviving this > > Sophie > > On Wed, Jan 13, 2021 at 7:47 AM Leah Thomas wrote: > > > Hey all, > > > > Bringing this

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2021-01-13 Thread Sophie Blee-Goldman
That sounds good to me. Thanks for reviving this Sophie On Wed, Jan 13, 2021 at 7:47 AM Leah Thomas wrote: > Hey all, > > Bringing this back up for discussion. > > It seems like the next steps are to: > 1. rename the config "window.size.ms" > 2. ensure that users set window size EITHER through

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2021-01-13 Thread Leah Thomas
Hey all, Bringing this back up for discussion. It seems like the next steps are to: 1. rename the config "window.size.ms" 2. ensure that users set window size EITHER through the config OR through the constructor. On this note, it may make sense to remove the default for the `window.size.ms`

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-29 Thread Sophie Blee-Goldman
There are two cases where you need to specify the window size -- directly using a Consumer (eg the console consumer) or reading as an input topic within Streams. We need a config for the first case, since you can't pass a Deserializer object to the console consumer. In the Streams case, the

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-08 Thread Matthias J. Sax
From my understanding, the KIP aims for the case when a user does not control the code, eg, when using the command line consumer (or similar tools). If the user writes code, we should always encourage her to instantiate the deserializer explicitly and not relying on reflection+config. I also

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-08 Thread Guozhang Wang
Hi Sophie, Seems I do have some mis-understanding of the KIP's motivation here :) Just for clarification my reasoning is that: 1) today Streams itself never uses a windowed deserializer itself since its built-in operators only need the serializer and users do not need to override it, plus

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-08 Thread Leah Thomas
Hey Sophie, Are you advocating that we change the name back to *window.size.ms *? I wasn't thinking that by including the config we would steer people away from using the constructors that pass in a window size, so in that sense I suppose the config isn't "default." I

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-08 Thread Sophie Blee-Goldman
Hey Guozhang & Leah, I want to push back a bit on the assumption that we would fall back on this config in the case of an unspecified window size in a Streams serde. I don't think it should be a default at all, either in name or in effect. To borrow the rhetorical question that John raised

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-08 Thread Guozhang Wang
Thanks Leah, I'm +1 on the KIP proposal. Guozhang On Tue, Sep 8, 2020 at 10:40 AM Leah Thomas wrote: > Hi Guozhang, > > Yes, the config would read them as a single window size. I think this > relates to John's comments about having variably sized windows, which this > config doesn't handle. I

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-08 Thread Leah Thomas
Hi Guozhang, Yes, the config would read them as a single window size. I think this relates to John's comments about having variably sized windows, which this config doesn't handle. I like the name change and updated the wiki to reflect that, and to clarify that the default value will still be

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-08 Thread Guozhang Wang
Hello Leah, Thanks for initiating this. I just have one minor clarification question here: the config "window.size.ms" seems to be used as the default window size when reading from a topic that represents windowed records right? I.e. if there are multiple topics that represent windowed records

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-08 Thread Leah Thomas
Hey all, We should be good to wrap up voting now that the discussion has been resolved. Cheers, Leah On Wed, Sep 2, 2020 at 7:23 PM Matthias J. Sax wrote: > +1 (binding) > > On 8/26/20 8:02 AM, John Roesler wrote: > > Hi all, > > > > I've just sent a new message to the DISCUSS thread. We > >

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-09-02 Thread Matthias J. Sax
+1 (binding) On 8/26/20 8:02 AM, John Roesler wrote: > Hi all, > > I've just sent a new message to the DISCUSS thread. We > forgot to include the Scala API in the proposal. > > Thanks, > -John > > On Mon, 2020-08-24 at 18:00 -0700, Sophie Blee-Goldman > wrote: >> Thanks for the KIP! +1

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-08-26 Thread John Roesler
Hi all, I've just sent a new message to the DISCUSS thread. We forgot to include the Scala API in the proposal. Thanks, -John On Mon, 2020-08-24 at 18:00 -0700, Sophie Blee-Goldman wrote: > Thanks for the KIP! +1 (non-binding) > > Sophie > > On Mon, Aug 24, 2020 at 5:06 PM John Roesler

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-08-24 Thread Sophie Blee-Goldman
Thanks for the KIP! +1 (non-binding) Sophie On Mon, Aug 24, 2020 at 5:06 PM John Roesler wrote: > Thanks Leah, > I’m +1 (binding) > > -John > > On Mon, Aug 24, 2020, at 16:54, Leah Thomas wrote: > > Hi everyone, > > > > I'd like to kick-off the vote for KIP-659: Improve > >

Re: [VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-08-24 Thread John Roesler
Thanks Leah, I’m +1 (binding) -John On Mon, Aug 24, 2020, at 16:54, Leah Thomas wrote: > Hi everyone, > > I'd like to kick-off the vote for KIP-659: Improve > TimeWindowedDeserializer > and TimeWindowedSerde to handle window size. >

[VOTE] KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size

2020-08-24 Thread Leah Thomas
Hi everyone, I'd like to kick-off the vote for KIP-659: Improve TimeWindowedDeserializer and TimeWindowedSerde to handle window size. https://cwiki.apache.org/confluence/display/KAFKA/KIP-659%3A+Improve+TimeWindowedDeserializer+and+TimeWindowedSerde+to+handle+window+size Thanks, Leah