-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-34969][cdc-cli]Add support for both new and old Flink config files in Flink… #3194
[FLINK-34969][cdc-cli]Add support for both new and old Flink config files in Flink… #3194
Conversation
@skymilong Thanks for the PR! According to the code contribution rule of Apache Flink, could you create an issue on Jira for this and include the Jira ID in the PR title and commit message? You can take #3160 as an example. And the PR has code formatting issue. Please run Thanks! |
Thanks a bunch for your help. This is my first rodeo contributing code on GitHub, so I'm still trying to get the hang of things. I'll whip up a Jira issue like you suggested and pop the ID into the PR and commit messages. Also, I'll iron out those code formatting kinks. I'll get on it pronto and get the PR updated. |
@skymilong Welcome to the community! Feel free to ask any questions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skymilong Thanks for the update! I left a comment.
flink-cdc-cli/src/main/java/org/apache/flink/cdc/cli/utils/FlinkEnvironmentUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skymilong Thanks for the update! I left some comments.
flink-cdc-cli/src/main/java/org/apache/flink/cdc/cli/utils/FlinkEnvironmentUtils.java
Outdated
Show resolved
Hide resolved
flink-cdc-cli/src/main/java/org/apache/flink/cdc/cli/utils/ConfigurationUtils.java
Outdated
Show resolved
Hide resolved
flink-cdc-cli/src/main/java/org/apache/flink/cdc/cli/utils/ConfigurationUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skymilong Thanks for the update! I left a comment.
flink-cdc-common/src/main/java/org/apache/flink/cdc/common/utils/MemorySize.java
Outdated
Show resolved
Hide resolved
@skymilong Thanks for the update! Could you rebase the latest master branch? I just merged a commit for fixing unstable CI tests |
- Add a new utility class `YamlParserUtils` for parsing YAML files. This class leverages the `snakeyaml` library to provide robust and flexible parsing capabilities. - Rename the method `loadMapFormattedConfig` to `loadConfigFile` in the `ConfigurationUtils` class to better reflect its purpose. - Improve the `loadConfigFile` method to handle configurations that include lists, in addition to maps. Note: This commit adds a new dependency on `snakeyaml` version 2.6.
f5ac033
to
a4452ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skymilong Thanks for the update! LGTM
… files in Flink CDC (apache#3194)
This commit introduces a simple logic check within the Flink CDC codebase to enable support for both the new (
config.yaml
) and old (flink-conf.yaml
) Flink configuration file standards. By utilizing theFiles.exists
method, Flink CDC now first checks for the presence ofconfig.yaml
in the system. Ifconfig.yaml
is found, it is used as the configuration file. Otherwise, Flink CDC falls back to usingflink-conf.yaml
. This enhancement ensures that Flink CDC can seamlessly transition between different versions of Flink, maintaining backward compatibility and facilitating easier upgrades for users.