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 > >>> > >> > > >
