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
