XComp commented on a change in pull request #13747:
URL: https://github.com/apache/flink/pull/13747#discussion_r516691971



##########
File path: 
flink-yarn/src/main/java/org/apache/flink/yarn/entrypoint/YarnSessionClusterEntrypoint.java
##########
@@ -73,7 +76,17 @@ public static void main(String[] args) {
                        LOG.warn("Could not log YARN environment information.", 
e);
                }
 
-               Configuration configuration = 
YarnEntrypointUtils.loadConfiguration(workingDirectory, env);
+               final CommandLineParser<Configuration> commandLineParser = new 
CommandLineParser<>(new DynamicParametersConfigurationParserFactory());
+               Configuration dynamicParameters = null;
+               try {
+                       dynamicParameters = commandLineParser.parse(args);
+               } catch (FlinkParseException e) {
+                       LOG.error("Could not parse command line arguments {}.", 
args, e);
+                       
commandLineParser.printHelp(YarnSessionClusterEntrypoint.class.getSimpleName());
+                       System.exit(1);
+               }

Review comment:
       Ok, fair enough. I addressed the comments and pushed the changes. FYI: I 
also moved `ClusterEntrypointUtils` into the package of `ClusterEntrypoint` 
(i.e. `org.apache.flink.runtime.entrypoint`) since I wanted to use 
`ClusterEntrypoint.STARTUP_FAILURE_RETURN_CODE` in the newly introduced utility 
method.
   
   Alternatively, I could have made 
`ClusterEntrypoint.STARTUP_FAILURE_RETURN_CODE` public. But moving 
`ClusterEntrypointUtils` felt like the right thing to do considering that other 
utility classes (e.g. `ClusterEntrypointExceptionUtils`) are also located in 
`org.apache.flink.runtime.entrypoint`. Any objections against that?




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


Reply via email to