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
> 

Reply via email to