PS-- I was re-reading the PR that originated this discussion and realized
that `window.inner.serde.class` is used here
<https://github.com/a0x8o/kafka/blob/master/streams/src/main/java/org/apache/kafka/streams/kstream/TimeWindowedSerializer.java#L44>
in KStreams. This goes against removing it, yes?
On Mon, Mar 11, 2024 at 10:40 AM Lucia Cerchie <lcerc...@confluent.io>
wrote:
> 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>
>
--
[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>
