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 > > > >
