Thanks for the efforts, Rui and Xuannan.
I think it's a good idea to migrate string-key configuration accesses to
ConfigOption-s in general.
I have a few suggestions / questions regarding the FLIP.
1. I think the default value for `TASK_MANAGER_LOG_PATH_KEY` should be "no
default". We can explain in the description that if not configured,
`System.getProperty("log.file")` will be used, but that is not a default
value.
2. I wonder if the following string-keys can be simply removed? They are
neither set by Flink, nor documented anywhere (AFAIK) so that users know
how to set them. All of them were introduced a long time ago, require
significant knowledge and familiarity about Flink internals and low level
details in order to use, and some of them are even private. So I wonder if
we can simply mark them as deprecated and remove in 2.0.
- ConfigConstants.YARN_CONTAINER_START_COMMAND_TEMPLATE
- FileInputFormat.FILE_PARAMETER_KEY
- FileInputFormat.ENUMERATE_NESTED_FILES_FLAG
- FileOutputFormat.FILE_PARAMETER_KEY
- BinaryInputFormat.BLOCK_SIZE_PARAMETER_KEY
- BinaryOutputFormat.BLOCK_SIZE_PARAMETER_KEY
3. Simply saying "getting / setting value with string key is discouraged"
in JavaDoc of get/setString is IMHO a bit confusing. People may have the
question why would we keep the discouraged interfaces at all. I would
suggest the following:
```
We encourage users and developers to always use ConfigOption for getting /
setting the configurations if possible, for its rich description, type,
default-value and other supports. The string-key-based getter / setter
should only be used when ConfigOption is not applicable, e.g., the key is
programmatically generated in runtime.
```
WDYT?
Best,
Xintong
On Tue, Dec 26, 2023 at 4:12 PM Rui Fan <1996fan...@gmail.com> wrote:
> Hi all,
>
> Xuannan(cced) and I would like to start a discussion on FLIP-405:
> FLIP-405: Migrate string configuration key to ConfigOption[1].
>
> As Flink progresses to 2.0, we want to enhance the user experience
> with the existing configuration. In FLIP-77, Flink introduced ConfigOption
> with DataType and strongly encourage users to utilize ConfigOption
> instead of string keys for accessing and setting Flink configurations.
> Presently, many string configuration keys have been deprecated and
> replaced with ConfigOptions; however, some string configuration
> keys are still in use.
>
> To ensure a better experience with the existing configuration in Flink
> 2.0,
> this FLIP will migrate all user-facing string configuration keys to
> ConfigOptions.
> Additionally, we want to modify the Configuration infrastructure to
> promote the use of ConfigOption over string configuration keys
> among developers and users. It's mentioned in a preview thread[2].
>
> Looking forward to everyone's feedback and suggestions, thank you!
>
> [1] https://cwiki.apache.org/confluence/x/6Yr5E
> [2] https://lists.apache.org/thread/zzsf7glfcdjcjm1hfo1xdwc6jp37nb3m
>
> Best,
> Rui
>
