Thanks for your reply Zhu Zhu. I guess, if we don't see any value in aligning the parameter names (I don't have a strong argument either aside from "it looks nicer"), there wouldn't be a need to add it to the guidelines as well.
Sorry for not responding right away. I did a bit of research on the AuthInfo configuration parameter (server side authorization [1], SO thread on utilizing curator's authorization API [2]). It looks like using String#getBytes() is the valid approach to configure this. So, in this way, I don't have anything to add to this FLIP proposal. +1 LGTM [1] https://cwiki.apache.org/confluence/display/ZOOKEEPER/Client-Server+mutual+authentication [2] https://stackoverflow.com/questions/40427700/using-acl-with-curator On Wed, Jan 24, 2024 at 12:35 PM Zhu Zhu <reed...@gmail.com> wrote: > @Matthias > Thanks for raising the question. > > AFAIK, there is no guide of this naming convention yet. But +1 to add this > naming > convention in Flink code style guide. So that new configuration names can > follow > the guide. > > However, I tend to not force the configuration name alignment in Flink 2.0. > It does not bring obvious benefits to users but will increase the > migration cost. > And the feature freeze of 1.19 is coming soon. I think we can add aligned > key > names for those exceptional config options in 1.20, but remove the old > keys in > later major versions. > > Thanks, > Zhu > > Matthias Pohl <matthias.p...@aiven.io> 于2024年1月23日周二 19:45写道： > >> - Regarding names: sure it totally makes sense to follow the kebab case >>> and Flip has reflected the change. >>> Regarding the convention, Flink has this widely used configuration >>> storageDir, which doesn't follow the kebab rule and creates some confusion. >>> IMHO it would be valuable to add a clear guide. >> >> >> Ah true, I should have checked the HA-related parameters as well. >> Initially, I just briefly skimmed over a few ConfigOptions names. >> >> @Zhu Zhu Is the alignment of the configuration parameter names also part >> of the 2.0 efforts that touch the Flink configuration? Is there a guideline >> we can follow here which is future-proof in terms of parameter naming? >> >> - I am considering calling the next method from the Curator framework: >>> authorization(List<AuthInfo>) [2]. I have added necessary details regarding >>> Map<String, String> -> List(AuthInfo) conversion, taking into account that >>> AuthInfo has a constructor with String, byte[] parameters. >>> >> >> The update in the FLIP looks good to me. >> >> - Good point. Please let me know if I am missing something, but it seems >>> that we already can influence ACLProvider for Curator in Flink with >>> high-availability.zookeeper.client.acl [2] . The way it is done currently >>> is translation of the predefined constant to some predefined ACL Provider >>> [3]. I do not see if we can add something to the current FLIP. I suppose >>> that eventual extension of the supported ACLProvider would be >>> straightforward and could be done outside of the current Flip as soon >>> as concrete use-case requirements arise. >> >> >> Thanks for the pointer. My concern is just that, we might have to >> consider certain formats for the AuthInfo to be aligned with ACLProviders. >> >> @Marton: I know that ZooKeeper is probably a bit unrelated to FLIP-211 >> [1] but since you worked on the Kerberos delegation token provider: Is >> there something to consider for the ZK Kerberos integration? Maybe, you can >> help us out. >> >> Matthias >> >> [1] >> https://cwiki.apache.org/confluence/display/FLINK/FLIP-211%3A+Kerberos+delegation+token+framework >> >> On Mon, Jan 22, 2024 at 6:55 PM Alex Nitavsky <alexnitav...@gmail.com> >> wrote: >> >>> Hello Matthias, >>> >>> Thanks a lot for the feedback. >>> >>> - Regarding names: sure it totally makes sense to follow the kebab case >>> and Flip has reflected the change. >>> Regarding the convention, Flink has this widely used configuration >>> storageDir, which doesn't follow the kebab rule and creates some confusion. >>> IMHO it would be valuable to add a clear guide. >>> >>> - I am considering calling the next method from the Curator framework: >>> authorization(List<AuthInfo>) [2]. I have added necessary details regarding >>> Map<String, String> -> List(AuthInfo) conversion, taking into account that >>> AuthInfo has a constructor with String, byte[] parameters. >>> >>> - Good point. Please let me know if I am missing something, but it seems >>> that we already can influence ACLProvider for Curator in Flink with >>> high-availability.zookeeper.client.acl [2] . The way it is done currently >>> is translation of the predefined constant to some predefined ACL Provider >>> [3]. I do not see if we can add something to the current FLIP. I suppose >>> that eventual extension of the supported ACLProvider would be >>> straightforward and could be done outside of the current Flip as soon >>> as concrete use-case requirements arise. >>> >>> Kind Regards >>> Oleksandr >>> >>> [1] >>> https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#authorization(java.util.List)https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#authorization(java.util.List) >>> <https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#authorization(java.util.List)> >>> [2] >>> https://nightlies.apache.org/flink/flink-docs-release-1.18/docs/deployment/config/#high-availability-zookeeper-client-acl >>> [3] >>> https://github.com/apache/flink/blob/master/flink-runtime/src/main/java/org/apache/flink/runtime/util/ZooKeeperUtils.java#L208 >>> >>> >>> >>> On Mon, Jan 22, 2024 at 1:10 PM Matthias Pohl >>> <matthias.p...@aiven.io.invalid> wrote: >>> >>>> Hi Alex, >>>> thanks for bringing this up and sorry for jumping into the discussion >>>> that >>>> late. Exposing additional curator configuration parameters through >>>> Flink's >>>> config sounds reasonable. Overall, I am fine with the selection of >>>> parameters. My judgement is entirely based on the curator >>>> description, though. If someone has more experience with >>>> configuring/deploying ZooKeeper; it would be great to get more insights. >>>> >>>> Here are a few remarks: >>>> - You propose to use camel-case parameter names. I tried to find some >>>> documentation where we decide on the actual formatting for configuration >>>> parameters. Unfortunately, I didn't find any. The Flink configuration >>>> appears to follow kebap-casing, though. We might want to use this >>>> format in >>>> this FLIP as well. >>>> >>>> A bit out-of-scope for this FLIP: Is the configuration parameter format >>>> documented somewhere? Should we add such a rule to the coding >>>> conventions >>>> [1]. What do others think? >>>> >>>> - You mention the authentication parameter which expects byte[]. You >>>> suggest using getBytes() to transform the value from the Flink >>>> configuration "Map<String, String>" to what Curator expects. Can you >>>> elaborate in the FLIP a bit more how the overall configuration would >>>> look >>>> like in the end? I assume you refer to ConfigOptions#mapType when >>>> talking >>>> about Map<String, String>? Do you want to allow multiple AuthInfo >>>> records >>>> to be configurable? >>>> >>>> - In [2], I found an example where they also suggest utilizing the >>>> AclProvider for allowing write access. This wouldn't be necessary in our >>>> case because the credentials are managed by an outside process, I >>>> guess? Or >>>> do we need to look into supporting AclProviders as well? >>>> >>>> Best, >>>> Matthias >>>> >>>> [1] >>>> >>>> https://flink.apache.org/how-to-contribute/code-style-and-quality-formatting/#naming >>>> [2] https://stackoverflow.com/a/55171761 >>>> >>>> >>>> On Mon, Jan 8, 2024 at 9:46 AM Alex Nitavsky <alexnitav...@gmail.com> >>>> wrote: >>>> >>>> > Thanks Zhanghao Chen for the feedback. >>>> > >>>> > In general right now there is no way to authorise Flink via the >>>> Curator >>>> > framework, which is probably an important feature which is missing. >>>> Since >>>> > public configuration change requires a Flip, current Flip is >>>> proposing to >>>> > add several more potentially important Curator configurations. >>>> > >>>> > Kind regards >>>> > Oleksandr >>>> > >>>> > On Thu, Jan 4, 2024 at 4:57 AM Zhanghao Chen < >>>> zhanghao.c...@outlook.com> >>>> > wrote: >>>> > >>>> > > Thanks for driving this. I'm not familiar with the listed advanced >>>> > Curator >>>> > > configs, but the previous added switch for disabling ensemble >>>> tracking >>>> > [1] >>>> > > saved us when deploying Flink in a cloud env where ZK can only be >>>> > > accessible via URLs. That being said, +1 for the overall idea, these >>>> > > configs may help users in certain scenarios sooner or later. >>>> > > >>>> > > [1] https://issues.apache.org/jira/browse/FLINK-31780 >>>> > > >>>> > > Best, >>>> > > Zhanghao Chen >>>> > > ________________________________ >>>> > > From: Alex Nitavsky <alexnitav...@gmail.com> >>>> > > Sent: Thursday, December 14, 2023 21:20 >>>> > > To: dev@flink.apache.org <dev@flink.apache.org> >>>> > > Subject: [DISCUSS] FLIP-402: Extend ZooKeeper Curator configurations >>>> > > >>>> > > Hi all, >>>> > > >>>> > > I would like to start a discussion thread for: *FLIP-402: Extend >>>> > ZooKeeper >>>> > > Curator configurations *[1] >>>> > > >>>> > > * Problem statement * >>>> > > Currently Flink misses several Apache Curator configurations, which >>>> could >>>> > > be useful for Flink deployment with ZooKeeper as HA provider. >>>> > > >>>> > > * Proposed solution * >>>> > > We have inspected all possible options for Apache Curator and >>>> proposed >>>> > > those which could be valuable for Flink users: >>>> > > >>>> > > - high-availability.zookeeper.client.authorization [2] >>>> > > - high-availability.zookeeper.client.maxCloseWaitMs [3] >>>> > > - >>>> high-availability.zookeeper.client.simulatedSessionExpirationPercent >>>> > [4] >>>> > > >>>> > > The proposed way is to reflect those properties into Flink >>>> configuration >>>> > > options for Apache ZooKeeper. >>>> > > >>>> > > Looking forward to your feedback and suggestions. >>>> > > >>>> > > Kind regards >>>> > > Oleksandr >>>> > > >>>> > > [1] >>>> > > >>>> > > >>>> > >>>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-402%3A+Extend+ZooKeeper+Curator+configurations >>>> > > [2] >>>> > > >>>> > > >>>> > >>>> https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#authorization(java.lang.String,byte%5B%5D) >>>> > > [3] >>>> > > >>>> > > >>>> > >>>> https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#maxCloseWaitMs(int) >>>> > > [4] >>>> > > >>>> > > >>>> > >>>> https://curator.apache.org/apidocs/org/apache/curator/framework/CuratorFrameworkFactory.Builder.html#simulatedSessionExpirationPercent(int) >>>> > > >>>> > >>>> >>>
