-
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-35360] support Flink cdc pipeline Yarn application mode. #3599
base: master
Are you sure you want to change the base?
Conversation
hi @Mrart ,I merged your PR and tested it, and found the following problem which confuses me. Can you help me check it out?
Other non-flink-cdc tasks run normally |
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.
Thanks for @Mrart's contribution, just left some trivial comments.
return Optional.ofNullable(distJars.get(0)); | ||
} catch (IOException e) { | ||
LOG.error( | ||
"Get flink-cdc-dist.jar from Flink cdc home lib is : {} failed", |
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.
"Get flink-cdc-dist.jar from Flink cdc home lib is : {} failed", | |
"Failed to fetch Flink CDC dist jar from path {}", |
path -> | ||
path.getFileName() | ||
.toString() | ||
.matches("flink-cdc-dist-.*-.*\\.jar")) |
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.
Will this regex match release jars like flink-cdc-dist-3.0.0.jar
?
.toString() | ||
.matches("flink-cdc-dist-.*-.*\\.jar")) | ||
.forEach(path -> distJars.add(String.valueOf(path.toAbsolutePath()))); | ||
return Optional.ofNullable(distJars.get(0)); |
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.
distJars.get(0)
will never be null. If distJars
is empty, an IndexOutOfBoundsException
will be thrown instead of returning Optional.empty()
.
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.
Maybe we can add some test cases to verify this change?
support Flink cdc pipeline Yarn application mode.