Hi Rui, I'd like to understand why there is a strong requirement for these deprecated methods. The ConfigOption param methods help to do the type conversion so that users do not need to do it by themselves.
For the 2 reasons to keep these methods mentioned above: > 1. A lot of scenarios don't define the ConfigOption, they using String as the key and value directly, such as: StreamConfig, TaskConfig, DistributedCache, etc. These are internal usages and we can have an internal getter method for them. It is not a blocker of the deprecation, epsecially given that they are not standard configuration and are just using Configuration class for convenience. > 2. Some code wanna convert all keys or values, this converting is generic, so the getString(String key, String defaultValue) is needed. Such as: kubernetes-operator [3]. Not sure why it's required to convert all keys or values. If it is used to create strings for dynamic properties or config files to deploy jobs, toMap()/toFileWritableMap() may be a better choice. They are already used in this kind of scenarios. If it just needs to read some of the config, why not using the proposed way to read and parse the config? Pre-defined ConfigOptions are better for configuration maintenance, compared to arbitrary strings Thanks, Zhu Rui Fan <1996fan...@gmail.com> 于2023年12月13日周三 19:27写道： > Thanks Martijn for the quick clarification! > > I see Zhu Zhu and Junrui Lee are working on configuration related > work of Flink-2.0. I would cc them, and hear some thoughts from them. > > Best, > Rui > > On Wed, Dec 13, 2023 at 7:17 PM Martijn Visser <martijnvis...@apache.org> > wrote: > >> Hi Rui, >> >> I'm more wondering if part of the configuration layer changes would >> mean that these APIs would be removed in 2.0. Because if so, then I >> don't think we should remove the Deprecate annotation. But I have very >> little visibility on the plans for the configuration layer. >> >> Thanks, >> >> Martijn >> >> On Wed, Dec 13, 2023 at 12:15 PM Rui Fan <1996fan...@gmail.com> wrote: >> > >> > Hi Martijn, >> > >> > Thanks for your reply! >> > >> > I noticed the 2.0 is doing some work related to clean configuration. >> > But this discussion is different from other work. Most of other work >> > are deprecate some Apis in Flink-1.19 and remove them in Flink-2.0. >> > >> > This discussion is a series of methods have been marked to @Deprecate, >> > but they are still used so far. I propose remove the @Deprecate >> annotation >> > and keep these methods. >> > >> > In brief, I think some deprecated methods shouldn't be deprecated. >> > WDYT? >> > >> > Please correct me if I'm wrong, thanks~ >> > >> > Best, >> > Rui >> > >> > On Wed, Dec 13, 2023 at 7:07 PM Martijn Visser < >> martijnvis...@apache.org> >> > wrote: >> > >> > > Hi Rui, >> > > >> > > Are you thinking about this as part of Flink 2.0, since that has the >> > > goal to introduce a completely clean configuration layer? [1] >> > > >> > > Best regards, >> > > >> > > Martijn >> > > >> > > [1] https://cwiki.apache.org/confluence/display/FLINK/2.0+Release >> > > >> > > On Wed, Dec 13, 2023 at 11:28 AM Maximilian Michels <m...@apache.org> >> > > wrote: >> > > > >> > > > Hi Rui, >> > > > >> > > > +1 for removing the @Deprecated annotation from `getString(String >> key, >> > > > String defaultValue)`. I would remove the other typed variants with >> > > > default values but I'm ok with keeping them if they are still used. >> > > > >> > > > -Max >> > > > >> > > > On Wed, Dec 13, 2023 at 4:59 AM Rui Fan <1996fan...@gmail.com> >> wrote: >> > > > > >> > > > > Hi devs, >> > > > > >> > > > > I'd like to start a discussion to discuss whether Configuration >> > > supports >> > > > > getting value based on the String key. >> > > > > >> > > > > In the FLIP-77[1] and FLINK-14493[2], a series of methods of >> > > Configuration >> > > > > are marked as @Deprecated, for example: >> > > > > - public String getString(String key, String defaultValue) >> > > > > - public long getLong(String key, long defaultValue) >> > > > > - public boolean getBoolean(String key, boolean defaultValue) >> > > > > - public int getInteger(String key, int defaultValue) >> > > > > >> > > > > The java doc suggests using getString(ConfigOption, String) or >> > > > > getOptional(ConfigOption), it means using ConfigOption as key >> > > > > instead of String. >> > > > > >> > > > > They are depreated since Flink-1.10, but these methods still >> > > > > be used in a lot of code. I think getString(String key, String >> > > > > defaultValue) >> > > > > shouldn't be deprecated with 2 reasons: >> > > > > >> > > > > 1. A lot of scenarios don't define the ConfigOption, they using >> > > > > String as the key and value directly, such as: StreamConfig, >> > > > > TaskConfig, DistributedCache, etc. >> > > > > >> > > > > 2. Some code wanna convert all keys or values, this converting >> > > > > is generic, so the getString(String key, String defaultValue) is >> > > needed. >> > > > > Such as: kubernetes-operator [3]. >> > > > > >> > > > > Based on it, I have 2 solutions: >> > > > > >> > > > > 1. Removing the @Deprecated for these methods. >> > > > > >> > > > > 2. Only removing the @Deprecated for `public String >> getString(String >> > > key, >> > > > > String defaultValue)` >> > > > > and delete other getXxx(String key, Xxx defaultValue) directly. >> > > > > They have been depreated 8 minor versions ago. In general, the >> > > > > getString can replace getInteger, getBoolean, etc. >> > > > > >> > > > > I prefer solution1, because these getXxx methods are used for now, >> > > > > they are easy to use and don't bring large maintenance costs. >> > > > > >> > > > > Note: The alternative to public String getString(String key, >> String >> > > > > defaultValue) >> > > > > is Configuration.toMap. But the ease of use is not very >> convenient. >> > > > > >> > > > > Looking forward to hear more thoughts about it! Thank you~ >> > > > > Also, very much looking forward to feedback from Dawid, the >> author of >> > > > > FLIP-77. >> > > > > >> > > > > [1] https://cwiki.apache.org/confluence/x/_RPABw >> > > > > [2] https://issues.apache.org/jira/browse/FLINK-14493 >> > > > > [3] >> > > > > >> > > >> https://github.com/apache/flink-kubernetes-operator/pull/729/files#r1424811105 >> > > > > >> > > > > Best, >> > > > > Rui >> > > >> >
