Hi Cheng Pan, Are you still interested in following up with this KIP? This is a useful feature so it would be nice to get it in Kafka.
Thanks, Mickael On Wed, Aug 17, 2022 at 1:39 PM Mickael Maison <mickael.mai...@gmail.com> wrote: > > Hi Cheng Pan, > > Thanks for the updates. I have a few more questions: > > 1) Since this KIP is targeting JsonConverter, I think we should add > this new config to JsonConverterConfig instead of ConverterConfig. > > 2) What about renaming the new field to something like > "use.null.for.optional.fields"? > > Thanks, > Mickael > > On Thu, May 5, 2022 at 3:19 PM Cheng Pan <cheng...@apache.org> wrote: > > > > Update PR links > > > > [1] https://github.com/apache/kafka/pull/12126 > > > > On 2022/05/05 13:16:59 Cheng Pan wrote: > > > Hi Mickael, > > > > > > Thanks for ping me. > > > > > > I updated the KIP-581 description to narrow the change scope to address > > > this specific issue, and raised a WIP PR[1] to implement it. > > > > > > Please let me know if you have any concerns. > > > > > > [1] https://github.com/apache/kafka/pull/12126 > > > > > > Thanks, > > > Cheng Pan > > > > > > On 2022/05/02 12:57:25 Mickael Maison wrote: > > > > Hi Cheng Pan, > > > > > > > > Thanks for raising this KIP, this would be a useful improvement! > > > > > > > > You never started a VOTE thread for this KIP. Are you interested in > > > > finishing up this work? > > > > If so, based on the discussion, I think you can open a VOTE thread as > > > > described in > > > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#KafkaImprovementProposals-Process > > > > If not, it's not a problem, someone else can volunteer to pick it up. > > > > > > > > Please let us know. > > > > > > > > Thanks, > > > > Mickael > > > > > > > > On Sat, Aug 8, 2020 at 11:05 AM Ruslan Gibaiev <r.giba...@gmail.com> > > > > wrote: > > > > > > > > > > Hello guys. > > > > > Proposed PR seems to be fixing the issue in a backward-compatible way. > > > > > Let's please move forward with it. Would be great to see it included > > > > > into next Kafka release > > > > > Thank you > > > > > > > > > > On 2020/07/29 02:49:07, "379377...@qq.com" <379377...@qq.com> wrote: > > > > > > Hi Chris, > > > > > > > > > > > > Thanks for your good suggestion, the KIP document and draft PR has > > > > > > been updated, please review again. > > > > > > > > > > > > And I found due to my misoperation, the mail thread has been > > > > > > broken, no idea how to fix it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > Cheng Pan > > > > > > > > > > > > From: Christopher Egerton > > > > > > Date: 2020-05-04 10:53 > > > > > > To: dev > > > > > > Subject: Re: [Discuss] KIP-581: Value of optional null field which > > > > > > has default value > > > > > > Hi Cheng, > > > > > > > > > > > > I think refactoring that method should be fine (if maybe a little > > > > > > painful); > > > > > > the method itself is private and all places that it's invoked > > > > > > directly are > > > > > > either package-private or non-static, so it shouldn't affect any of > > > > > > the > > > > > > public methods of the JSON converter to change "convertToConnect" > > > > > > to be > > > > > > non-static. Even if it did, the only parts of the JSON converter > > > > > > that are > > > > > > public API (and therefore officially subject to concerns about > > > > > > compatibility) are the methods it implements that satisfy the > > > > > > "Converter" > > > > > > and "HeaderConverter" interfaces. > > > > > > > > > > > > Would you mind explicitly specifying in the KIP that the new > > > > > > property will > > > > > > be added for the JSON converter only, and that it will affect both > > > > > > serialization and deserialization? > > > > > > > > > > > > Cheers, > > > > > > > > > > > > Chris > > > > > > > > > > > > On Tue, Apr 28, 2020 at 10:52 AM 379377944 <379377...@qq.com> wrote: > > > > > > > > > > > > > Hi Chris, > > > > > > > > > > > > > > > > > > > > > Thanks for your reminder, the original implement is deprecated, I > > > > > > > just > > > > > > > update the JIRA with the new > > > > > > > PR link: https://github.com/apache/kafka/pull/8575 > > > > > > > > > > > > > > > > > > > > > As question 2), I agree with you that we should consider both > > > > > > > serialization and deserialization, and as you said, I only > > > > > > > implement the > > > > > > > serialization now. This is because the original serde implement > > > > > > > is not > > > > > > > symmetrical, the convertToConnect is a static method and can’t > > > > > > > access the > > > > > > > field in JsonConverter > > > > > > > instance, maybe I should do some refactoring to implement the > > > > > > > deserialization. > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > Cheng Pan > > > > > > > Original Message > > > > > > > Sender: Christopher Egerton<chr...@confluent.io> > > > > > > > Recipient: dev<dev@kafka.apache.org> > > > > > > > Date: Wednesday, Apr 15, 2020 02:28 > > > > > > > Subject: Re: [Discuss] KIP-581: Value of optional null field > > > > > > > which has > > > > > > > default value > > > > > > > > > > > > > > > > > > > > > Hi Cheng, Thanks for the KIP! I really appreciate the care that > > > > > > > was taken > > > > > > > to ensure backwards compatibility for existing users, and the > > > > > > > minimal > > > > > > > changes to public interface that are suggested to address this. I > > > > > > > have two > > > > > > > quick requests for clarification: 1) Where is the proposed > > > > > > > "accept.optional.null" property going to apply? It's hinted that > > > > > > > it'll take > > > > > > > effect on the JSON converter but not actually called out > > > > > > > anywhere. 2) > > > > > > > Assuming this takes effect on the JSON converter, is the intent > > > > > > > to alter > > > > > > > the semantics for both serialization and deserialization? The > > > > > > > code snippet > > > > > > > from the JSON converter that's included in the KIP comes from the > > > > > > > "convertToJson" method, which is used for serialization. However, > > > > > > > based on > > > > > > > https://github.com/apache/kafka/blob/ea47a885b1fe47dfb87c1dc86db1b0e7eb8a273c/connect/json/src/main/java/org/apache/kafka/connect/json/JsonConverter.java#L712-L713 > > > > > > > it looks like the converter also inserts the default value for > > > > > > > optional-but-null data during deserialization. Thanks again for > > > > > > > the KIP! > > > > > > > Cheers, Chris On Wed, Mar 18, 2020 at 12:00 AM Cheng Pan > > > > > > > <379377...@qq.com> > > > > > > > wrote: > Hi all, > > I'd like to use this thread to discuss > > > > > > > KIP-581: Value > > > > > > > of optional null > field which has default value, please see > > > > > > > detail at: > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value > > > > > > > > > > There are some previous discussion at: > > > > > > > > https://github.com/apache/kafka/pull/7112 > > > I'm a beginner > > > > > > > for apache > > > > > > > project, please let me know if I did any thing > wrong. > > > > > > > > > > Best regards, > > > > > > > > Cheng Pan > > > > > > > > > > > > >
