Sophie, I'll add a paragraph about removing `windowed.inner.serde.class` to
the KIP. I'll also add putting it in the `TimeWindowedSerde` class with
some add'tl guidance on the docs addition.
Also, I double-checked setting window.size.ms on the client and it doesn't
throw an error at all, in response to Matthias's question. Changing the KIP
in response to that.
On Sun, Mar 10, 2024 at 6:04 PM Sophie Blee-Goldman <sop...@responsive.dev>
wrote:
> 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
> > >>
> >
>
--
[image: Confluent] <https://www.confluent.io>
Lucia Cerchie
Developer Advocate
Follow us: [image: Blog]
<https://www.confluent.io/blog?utm_source=footer&utm_medium=email&utm_campaign=ch.email-signature_type.community_content.blog>[image:
Twitter] <https://twitter.com/ConfluentInc>[image: Slack]
<https://slackpass.io/confluentcommunity>[image: YouTube]
<https://youtube.com/confluent>
[image: Try Confluent Cloud for Free]
<https://www.confluent.io/get-started?utm_campaign=tm.fm-apac_cd.inbound&utm_source=gmail&utm_medium=organic>
