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
