Leah, thanks for the KIP. It looks good to me overall, just following up on @br...@confluent.io <br...@confluent.io> 's question about exception: what about using the `TopologyException` class? I know that currently it is only thrown during the topology parsing phase, not at the streams construction, but I feel we can extend its scope to cover both topology building and streams object (i.e. taking the topology and the config) construction time as well since part of the construction is again to re-write / augment the topology.
Guozhang On Wed, May 19, 2021 at 8:44 AM Leah Thomas <ltho...@confluent.io.invalid> wrote: > Hi Sophie, > > Thanks for catching that. These are existing methods inside of > `StreamsConfig` that will return null (the new default) instead of byte > array serde (the old default). Both `StreamsConfig` and > `defaultKeySerde`/`defaultValueSerde` are public, so I assume these still > count as part of the public API. I updated the KIP to include this > information. > > Bruno - I was planning on including a specific message with the streams > exception to indicate that either a serde needs to be passed in or a > default needs to be set. I'm open to doing something more specific, > perhaps something like a serde exception? WDYT? I was hoping that with the > message the streams exception would still give enough information for users > to debug the problem. > > Still hoping for a short discussion (; but thanks for the input so far! > > Leah > > On Wed, May 19, 2021 at 3:00 AM Bruno Cadonna <cado...@apache.org> wrote: > > > Hey Leah, > > > > > what I think should be a small discussion > > > > Dangerous words, indeed! It seems like they trigger something in people > ;-) > > > > Jokes apart! > > > > Did you consider throwing a more specific exception instead of a > > StreamsException? Something that describes better the issue at hand. > > > > Best, > > Bruno > > > > > > On 19.05.21 01:19, Sophie Blee-Goldman wrote: > > >> > > >> what I think should be a small discussion > > > > > > > > > Dangerous words :P > > > > > > I'm all for the proposal but I do have one question about something in > > the > > > KIP. You list two methods called > > > defaultKeySerde() and defaultValueSerde() but it's not clear to me > where > > > these are coming from. Are they > > > new APIs you propose to add in this KIP? Are they existing methods in > the > > > public API which will now return > > > null, whereas they used to return the ByteArraySerde? If they're not a > > > public API then you can remove them > > > from the KIP, otherwise can you just update this section to clarify > what > > > class/file these belong to, etc? > > > > > > -Sophie > > > > > > On Tue, May 18, 2021 at 5:34 AM Leah Thomas > <ltho...@confluent.io.invalid > > > > > > wrote: > > > > > >> Hi all, > > >> > > >> I'd like to kick-off what I think should be a small discussion for > > KIP-741: > > >> Change default serde to be null. > > >> > > >> The wiki is here: > > >> > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-741%3A+Change+default+serde+to+be+null > > >> > > >> > > >> Thanks, > > >> Leah > > >> > > > > > > -- -- Guozhang
