Hi yuxia, Thank you for reviewing the code.
Specifically, when users set *'sink.partition-commit.policy.class.parameters=''*, the 'parameters' will not be null but will be an empty list. In this case, it may still invoke the constructor with empty arguments. To address this, maybe we can use the condition *'parameters != null && !parameters.isEmpty()'* to ensure that the 'parameters' list is not empty. What are your thoughts on this matter? I would appreciate your thoughts. : ) Best regards, Bangui Dunn yuxia <luoyu...@alumni.sjtu.edu.cn> 于2023年6月25日周日 11:07写道： > Hi, Bangui Dunn > Review done. > But please remember we should reach consensus about it in the dicsussion > before we can merge it. > Let's keep the discussion for a while to see any further feedback. > > Best regards, > Yuxia > > ----- 原始邮件 ----- > 发件人: "Dunn Bangui" <iceiceic...@gmail.com> > 收件人: "dev" <dev@flink.apache.org> > 发送时间: 星期日, 2023年 6 月 25日 上午 10:34:50 > 主题: Re: [DISCUSS] Flexible Custom Commit Policy with Parameter Passing > > Hi, Yuxia. > > Could you please provide me with an update on the review process? I am > eager to move forward and would appreciate any guidance or feedback you can > provide. > > Best regards, late Dragon Boat Festival blessing : ) > Bangui Dunn > > Gary Ccc <iceiceic...@gmail.com> 于2023年6月21日周三 14:44写道： > > > Hi, Yuxia. > > > > Thank you for your suggestion! I agree with your point and have made the > > necessary modification. > > If you have any additional feedback or further suggestions, please feel > > free to let me know. : ) > > > > Best regards, > > Bangui Dunn > > > > yuxia <luoyu...@alumni.sjtu.edu.cn> 于2023年6月21日周三 10:56写道： > > > >> Correct what I said in the previous email: > >> > >> "But will it better to make it asList ? Something > >> like:`.stringType().asList()`." => "But will it better to make it no > >> default value? Something like:`.stringType().asList().noDefaultValue`." > >> > >> Best regards, > >> Yuxia > >> > >> ----- 原始邮件 ----- > >> 发件人: "yuxia" <luoyu...@alumni.sjtu.edu.cn> > >> 收件人: "dev" <dev@flink.apache.org> > >> 发送时间: 星期三, 2023年 6 月 21日 上午 10:25:47 > >> 主题: Re: [DISCUSS] Flexible Custom Commit Policy with Parameter Passing > >> > >> Hi, Bangui Dunn. > >> Thanks for reaching us out. > >> Generally + 1 for the configuration. But will it better to make it > >> asList? Something like: > >> `.stringType().asList()`. > >> > >> Best regards, > >> Yuxia > >> > >> ----- 原始邮件 ----- > >> 发件人: "Gary Ccc" <iceiceic...@gmail.com> > >> 收件人: "dev" <dev@flink.apache.org> > >> 发送时间: 星期二, 2023年 6 月 20日 下午 5:44:10 > >> 主题: [DISCUSS] Flexible Custom Commit Policy with Parameter Passing > >> > >> To the Apache Flink Community Members, > >> I hope this email finds you well. I am writing to discuss a potential > >> improvement for the implementation of a custom commit policy in Flink. > >> > >> Background: > >> We have encountered challenges in utilizing a custom commit policy due > to > >> the inability to pass parameters. > >> This limitation restricts our ability to add additional functionality to > >> the commit policy, such as monitoring the files associated with each > >> commit. > >> > >> Purpose: > >> The purpose of this improvement is to allow the passing of parameters to > >> the custom PartitionCommitPolicy. By enabling parameter passing, users > can > >> extend the functionality of their custom commit policy. > >> > >> Example: > >> Suppose we have a custom commit policy called "MyPolicy" that requires > >> parameters such as "key" and "url" for proper functionality. > >> Currently, it is not possible to pass these parameters when using a > custom > >> commit policy. > >> However, by introducing the concept of > >> SINK_PARTITION_COMMIT_POLICY_CLASS_PARAMETERS, users can now pass > >> parameters in the following way: > >> > >> By adding SINK_PARTITION_COMMIT_POLICY_CLASS_PARAMETERS, you can pass > >> parameters when using a custom commit policy, for example > >> 'sink.partition-commit.policy.kind'='custom', > >> 'sink.partition-commit.policy.class’='MyPolicy', > >> 'sink.partition-commit.policy.class.parameters’=‘key;url' > >> > >> Eeffect: > >> By adding PartitionCommitPolicyFactory constructor, to ensure backward > >> compatibility for existing user programs. > >> > >> Code PR: > >> To support this improvement, I have submitted a pull request with the > >> necessary code changes. > >> You can find the details of the pull request at the following link: > >> https://github.com/apache/flink/pull/22831/files > >> > >> Best regards, > >> Bangui Dunn > >> > > >
