yuruguo commented on a change in pull request #14477:
URL: https://github.com/apache/flink/pull/14477#discussion_r553254765
##########
File path:
flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterClientFactory.java
##########
@@ -77,6 +78,8 @@ public ApplicationId getClusterId(Configuration
configuration) {
private YarnClusterDescriptor getClusterDescriptor(Configuration
configuration) {
final YarnClient yarnClient = YarnClient.createYarnClient();
final YarnConfiguration yarnConfiguration = new YarnConfiguration();
+
yarnConfiguration.addResource(HadoopUtils.getHadoopConfiguration(configuration));
+
yarnConfiguration.addResource(Utils.getYarnConfiguration(configuration));
Review comment:
If `HadoopUtils` provides both `getHadoopConfiguration` and
`getYarnConfiguration`, then it could be consistent and simplify code
https://github.com/apache/flink/pull/14477#discussion_r553198430.
But I am not sure if it is really appropriate and standardized to put
`getYarnConfiguration` in the `flink-hadoop-fs` module, because the
`flink-yarn` module itself has `Utils`, it may be more clear to use it directly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
