Thanks Chris for your quick reply. Your suggestions make sense, I amended the KIP and added a note to the class JavaDocs. Also added unit tests to the companion PR [https://github.com/apache/kafka/pull/14093], and will mark it as "Ready for Review" in a few.
Cheers From: dev@kafka.apache.org At: 07/25/23 10:42:58 UTC-4:00To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-959 Add BooleanConverter to Kafka Connect Hi Hector, Thanks for the KIP! Really appreciate the tight scope, hoping this will be easy to review :) I only have one question: it looks like our existing primitive converters (string converter + subclasses of NumberConverter) are hardcoded to play nicely with null values during deserialization by always providing an optional schema. If that's the intent with this KIP, can we specify that explicitly? (Could be as simple as saying "the schema returned during deserialization will always be an optional boolean schema" with a link to https://kafka.apache.org/35/javadoc/org/apache/kafka/connect/data/Schema.html#OP TIONAL_BOOLEAN_SCHEMA). I don't think we have to say anything else about null handling since FWICT the rest is already handled by the BooleanSerializer and BooleanDeserializer introduced in KIP-907. Cheers, Chris On Tue, Jul 25, 2023 at 9:52 AM Hector Geraldino (BLOOMBERG/ 919 3RD A) < hgerald...@bloomberg.net> wrote: > Hi everyone, > > I'd like to start a discussion of KIP-959, which aims to add a > BooleanConverter to Kafka Connect: > https://cwiki.apache.org/confluence/display/KAFKA/KIP-959%3A+Add+BooleanConverte r+to+Kafka+Connect > > This KIP is a counterpart of KIP-907: Add Boolean Serde to public > interface [ > https://cwiki.apache.org/confluence/display/KAFKA/KIP-907%3A+Add+Boolean+Serde+t o+public+interface], > which added Boolean SerDes to the Kafka serialization APIs. > > The scope of this KIP is very limited, and will help us close a small gap > that exists on the list of included converters for connect's "primitive" > types. > > Looking forward for your feedback. > > Regards, > Hector
