Yes, it's used inside `TimeWindowedSerializer` and actually also inside
`TimeWindowDeserializer`.
However, it does IMHO not change that we should remove it from
`StreamsConfig` because both configs are not intended to be used in Java
code... If one writes Java code, they should use
new TimeWindowedSerializer(Serializer)
new TimeWindowDeserializer(Deserializer, Long)
new TimeWindowSerdes(Serde, Long)
and thus they don't need either config.
The configs are only needed for command line tool, that create the
(de)serializer via reflection using the default constructor.
Does this make sense?
The only open question is really, if and where to add them... Strictly
speaking, we don't need either config as public variable as nobody
should use them in Java code. To me, it just feels right/better do make
them public for documentation purpose that these configs exists?
`inner.window.class.serde` has "serde" in it's name, so we could add it
to `TimeWindowSerdes`? For `window.size.ms`, it's only used by the
deserialize to maybe add it there? Just some ideas. -- Or we sidestep
this question and just don't add them; also fine with me.
-Matthias
On 3/11/24 10:53 AM, Lucia Cerchie wrote:
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>
