Hi Gunnar,
Thanks, this looks great. I'm ready to cast a non-binding on the vote
thread when it comes.
One small non-blocking nit: I like that you call out that the new
validation steps will take place when a connector gets registered or
updated. IMO this is important enough to be included in the "Public
Interfaces" section as that type of preflight check is arguably more
important than the PUT /connector-plugins/{name}/config/validate endpoint,
when considering that use of the validation endpoint is strictly opt-in,
but preflight checks for new connector configs are unavoidable (without
resorting to devious hacks like publishing directly to the config topic).
But this is really minor, I'm happy to +1 the KIP as-is.
Cheers,
Chris
On Tue, Dec 21, 2021 at 8:43 AM Gunnar Morling
<gunnar.morl...@googlemail.com.invalid> wrote:
> Hey Chris,
>
> Thanks a lot for reviewing this KIP and your comments! Some more answers
> inline.
>
> Am Di., 7. Dez. 2021 um 23:49 Uhr schrieb Chris Egerton
> <chr...@confluent.io.invalid>:
>
> > Hi Gunnar,
> >
> > Thanks for the KIP! The section on backwards compatibility is especially
> > impressive and was enjoyable to read.
> >
>
> Excellent, that's great to hear!
>
> Overall I like the direction of the KIP (and in fact just ran into a
> > situation yesterday where it would be valuable). I only have one major
> > thought: could we add similar validate methods for the Converter and
> > HeaderConverter interfaces? With KIP-769 [1], it looks like we'll have a
> > new Converter::config method, so if that makes it through, it should be a
> > matter of just adding the same methods to those interfaces as well
> > (although we may want to be tolerant of null ConfigDef objects being
> > returned from HeaderConverter::config since the Connect framework has not
> > been enforcing this requirement to date).
> >
>
> Yes, I think it's a good idea to expand the scope of the KIP to cover all
> these contracts. I have updated the KIP document accordingly.
>
> >
> > That aside, a few small nits:
> >
> > 1. The "This page is meant as a template" section can be removed :)
> > 2. The "Current Status" can be updated to "Under Discussion"
> > 3. Might want to add javadocs to the newly-proposed validate method (I'm
> > assuming they'll largely mirror the ones for the existing
> > Connector::validate method, but we may also want to add a {@since} tag or
> > some other information on which versions of Connect will leverage the
> > method).
> >
>
> Done.
>
> I will try and create a PR for this work in January next year.
>
> All the best,
>
> --Gunnar
>
> [1] -
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-769%3A+Connect+APIs+to+list+all+plugins+and+retrieve+their+configuration+definitions#KIP769:ConnectAPIstolistallpluginsandretrievetheirconfigurationdefinitions-PublicInterfaces
> > (section labeled "Converter interface"
> >
> > Cheers,
> >
> > Chris
> >
> > On Wed, Nov 24, 2021 at 11:32 AM Gunnar Morling
> > <gunnar.morl...@googlemail.com.invalid> wrote:
> >
> > > Hey all,
> > >
> > > I would like to propose a KIP for Apache Kafka Connect which adds
> > > validation support for SMT-related configuration options:
> > >
> > >
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-802%3A+Validation+Support+for+Kafka+Connect+SMT+Options
> > >
> > > This feature allows users to make sure an SMT is configured correctly
> > > before actually putting a connector with that SMT in place.
> > >
> > > Any feedback, comments, and suggestions around this proposal will
> > > be greatly appreciated.
> > >
> > > Thanks,
> > >
> > > --Gunnar
> > >
> >
>
