Thanks Hector! LGTM On Tue, Jul 25, 2023 at 11:47 AM Hector Geraldino (BLOOMBERG/ 919 3RD A) < hgerald...@bloomberg.net> wrote:
> 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 > > >
