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 >
