Re: [Discuss] KIP-581: Value of optional null field which has default value

2023-03-17 Thread Chris Egerton
LGTM On Fri, Mar 17, 2023, 08:26 Luke Chen wrote: > Thanks Mickael and Cheng! > This KIP LGTM! > > Thanks. > Luke > > On Fri, Mar 17, 2023 at 7:27 PM Mickael Maison > wrote: > > > Thanks Tom and Chris for your feedback! > > > > I agree one configuration is enough. I've renamed it to > > 'replac

Re: [Discuss] KIP-581: Value of optional null field which has default value

2023-03-17 Thread Luke Chen
Thanks Mickael and Cheng! This KIP LGTM! Thanks. Luke On Fri, Mar 17, 2023 at 7:27 PM Mickael Maison wrote: > Thanks Tom and Chris for your feedback! > > I agree one configuration is enough. I've renamed it to > 'replace.null.with.default'. > Since you both seemed happy with the KIP overall, I'

Re: [Discuss] KIP-581: Value of optional null field which has default value

2023-03-17 Thread Mickael Maison
Thanks Tom and Chris for your feedback! I agree one configuration is enough. I've renamed it to 'replace.null.with.default'. Since you both seemed happy with the KIP overall, I'll start a vote later today. Thanks, Mickael On Thu, Mar 16, 2023 at 7:08 PM Chris Egerton wrote: > > Hi Mickael, > >

Re: [Discuss] KIP-581: Value of optional null field which has default value

2023-03-16 Thread Chris Egerton
Hi Mickael, Tom got this part perfect so I'm just going to copy+paste: Thanks to Cheng for the KIP and to you for picking it up. I'm wondering why we need separate properties for serialization vs. deserialization? If the converter is being used by the Kafka Connect runtime, a given instance of it

Re: [Discuss] KIP-581: Value of optional null field which has default value

2023-03-16 Thread Tom Bentley
Hi Mickael, Thanks to Cheng for the KIP and to you for picking it up. My only comment (feel free to ignore) is about the names of the configs. Personally I don't think I'd correctly guess what "serialize.use.optional.null" meant. Something like "serialize.map.null.to.default" is much clearer to m

Re: [Discuss] KIP-581: Value of optional null field which has default value

2023-03-08 Thread Mickael Maison
Hi, This KIP has been staled for a long time. Since it would be a useful feature, I pinged Cheng about a month ago asking if he was planning to work on it again. I've not received a reply, so I've allowed myself to update the KIP (hopefully preserving the initial requirements) and would like to re

Re: [Discuss] KIP-581: Value of optional null field which has default value

2023-02-02 Thread Mickael Maison
> > > > > 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 deser

Re: [Discuss] KIP-581: Value of optional null field which has default value

2022-08-17 Thread Mickael Maison
ou > > > > > > > > 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 > > > >

Re: [Discuss] KIP-581: Value of optional null field which has default value

2022-05-05 Thread Cheng Pan
, 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

Re: [Discuss] KIP-581: Value of optional null field which has default value

2022-05-05 Thread Cheng Pan
t; > > > > > 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 > > >

Re: Re: [Discuss] KIP-581: Value of optional null field which has default value

2022-05-02 Thread Mickael Maison
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 fi

Re: Re: [Discuss] KIP-581: Value of optional null field which has default value

2020-08-08 Thread Ruslan Gibaiev
ou 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 >

Re: Re: [Discuss] KIP-581: Value of optional null field which has default value

2020-07-28 Thread 379377...@qq.com
: 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

Re: [Discuss] KIP-581: Value of optional null field which has default value

2020-05-03 Thread Christopher Egerton
; > Thanks, > Cheng Pan > Original Message > Sender: Christopher Egerton > Recipient: dev > 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

Re: [Discuss] KIP-581: Value of optional null field which has default value

2020-04-28 Thread 379377944
Sender: Christopher Egerton Recipient: dev 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

Re: [Discuss] KIP-581: Value of optional null field which has default value

2020-04-14 Thread Christopher Egerton
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.n