Hi everyone, On a semantic note, would it perhaps make more sense to rename them " uncaught." instead? These handlers are essentially the "last resort" exception handlers, because Exceptions can be caught *within* a component, e.g. a Deserializer can catch and handle an exception without the configured default.deserialization.exception.handler being invoked.
I think this would better align these with JVM norms, e.g. Thread#setUncaughtExceptionHandler, which behaves the same (albeit configured through code). Regards, Nick On Thu, 13 Jun 2024 at 10:22, Muralidhar Basani <muralidhar.bas...@aiven.io.invalid> wrote: > Thanks Matthias and Greg. > > Have updated the KIP based on Matthias's comments. > > >> 100: Config names are part of the public interface, and the KIP should > >> not say "none" in this section, but call out which configs are > >> deprecated and which ones are newly added. > > Updated kip. > > > >> 101: Nit. In "Propose Changes" there is the template placeholder text > >> > >>> Describe the new thing you want to do in appropriate detail. This may > be > >> fairly extensive and have large subsections of its own. Or it may be a > few > >> sentences. Use judgement based on the scope of the change. > >> > >> Similarly in "Test Plan" section > >> > > Updated kip. > > > >> 102: The "Deprecation" section should explain the behavior if both, old > >> and new configs, are set. > > Updated kip. I think a ConfigException has to be thrown here if both > configs are set. > > Thanks, > Murali > > On Thu, Jun 13, 2024 at 2:28 AM Matthias J. Sax <mj...@apache.org> wrote: > > > Thanks Greg, this is a valid idea. > > > > However, there was no demand in the past to allow for error handler > > callbacks in a fine grained manner, and I am frankly also not sure if it > > would make sense or would be required. > > > > Thus, I am not concerned about this case, and believe this KIP does make > > sense. > > > > Happy to change my mind if somebody has a different opinion. We could > > re-purpose this KIP to add per-topic error handler, too. > > > > > > > > -Matthias > > > > On 6/12/24 1:11 PM, Greg Harris wrote: > > > Hi Murali, > > > > > > Thanks for the KIP! > > > > > > I'm not familiar with Streams so I'll pose a general question, open for > > > anyone to answer: > > > > > > The configs that are being changed don't currently accept in-place > > > overwrites in the code, so the "default.*" prefix doesn't make sense. > > Could > > > there be a KIP to accept in-place overwrites in the future, such that > the > > > "default.*" prefix would make sense? > > > If so, this KIP would make that other KIP harder to implement, as we > > would > > > have already recommended everyone to move off of the "default.* prefix. > > Or > > > to put it another way, this KIP closes doors rather than opening them. > > > > > > Or at least that's how it looks to a Streams outsider. I'm happy to > defer > > > to the experts in this case :) > > > > > > Thanks, > > > Greg > > > > > > On Mon, Jun 10, 2024 at 3:47 PM Matthias J. Sax <mj...@apache.org> > > wrote: > > > > > >> Thanks for the KIP Murali, > > >> > > >> Overall LGTM. A few comments. > > >> > > >> > > >> > > >> 100: Config names are part of the public interface, and the KIP should > > >> not say "none" in this section, but call out which configs are > > >> deprecated and which ones are newly added. > > >> > > >> > > >> 101: Nit. In "Propose Changes" there is the template placeholder text > > >> > > >>> Describe the new thing you want to do in appropriate detail. This may > > be > > >> fairly extensive and have large subsections of its own. Or it may be a > > few > > >> sentences. Use judgement based on the scope of the change. > > >> > > >> Similarly in "Test Plan" section > > >> > > >> Please remove both :) > > >> > > >> > > >> 102: The "Deprecation" section should explain the behavior if both, > old > > >> and new configs, are set. > > >> > > >> > > >> Thanks a lot! > > >> > > >> > > >> -Matthias > > >> > > >> > > >> On 6/9/24 9:30 PM, Muralidhar Basani wrote: > > >>> Hello all, > > >>> > > >>> With this KIP > > >>> < > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1056%3A+Remove+%60default.%60+prefix+for+exception+handler+StreamsConfig > > >>> , > > >>> I would like to mention that a couple of exception handler configs in > > >>> StreamsConfig are defined as default configs, despite having no > > >> alternative > > >>> values. Hence would propose to deprecate them and introduce new > configs > > >>> without the 'default.' prefix. > > >>> > > >>> This KIP is briefly discussed here in the jira KAFKA-16853 > > >>> <https://issues.apache.org/jira/browse/KAFKA-16863> too. > > >>> > > >>> I would appreciate any feedback or suggestions you might have. > > >>> > > >>> Thanks, > > >>> Murali > > >>> > > >> > > > > > >
