Thanks for responding Matthias -- you got there first, but I was going to
say exactly the same thing as in your most reply. In other words, I see the
`windowed.inner.serde.class` as being in the same boat as the `
window.size.ms` config, so whatever we do with one we should do for the
other.

I do agree with removing these from StreamsConfig, but defining them in
ConsumerConfig feels weird as well. There's really no great answer here.

My only concern about adding it to the corresponding
serde/serializer/deserializer class is that it might be difficult for
people to find them. I generally assume that people tend not to look at the
serde/serializer/deserializer classes/implementations. But maybe in this
case, someone who actually needed these configs would be likely to be
motivated enough to find them by looking at the class? And with sufficient
documentation, it's likely not a problem. So, I'm +1 on putting it into the
TimeWindowedSerde class

(I would personally stick them into the serde class, rather than the
serializer and/or deserializer, but I could be convinced either way)

On Fri, Mar 1, 2024 at 3:00 PM Matthias J. Sax <mj...@apache.org> wrote:

> One more thought after I did some more digging on the related PR.
>
> Should we do the same thing for `windowed.inner.serde.class`?
>
>
> Both config belong to windowed serdes (which KS provide) but the KS code
> itself does never use them (and in fact, disallows to use them and would
> throw an error is used). Both are intended for plain consumer use cases
> for which the window serdes are used.
>
> The question to me is, should we add them back somewhere else? It does
> not really belong into `ConsumerConfig` either, but maybe we could add
> them to the corresponding serde or (de)serialize classes?
>
>
> -Matthias
>
>
> On 2/21/24 2:41 PM, Matthias J. Sax wrote:
> > Thanks for the KIP. Sounds like a nice cleanup.
> >
> >> window.size.ms  is not a true KafkaStreams config, and results in an
> >> error when set from a KStreams application
> >
> > What error?
> >
> >
> > Given that the configs is used by `TimeWindowedDeserializer` I am
> > wondering if we should additionally add
> >
> > public class TimeWindowedDeserializer {
> >
> >      public static final String WINDOW_SIZE_MS_CONFIG = "window.size.ms
> ";
> > }
> >
> >
> >
> > -Matthias
> >
> >
> > On 2/15/24 6:32 AM, Lucia Cerchie wrote:
> >> Hey everyone,
> >>
> >> I'd like to discuss KIP-1020
> >> <
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=290982804
> >,
> >> which would move to deprecate `window.size.ms` in `StreamsConfig`
> since `
> >> window.size.ms` is a client config.
> >>
> >> Thanks in advance!
> >>
> >> Lucia Cerchie
> >>
>

Reply via email to