[GitHub] [flink] Myracle commented on a diff in pull request #19772: [FLINK-27579][client] The param client.timeout can not be set by dyna…

[GitBox <gi...@apache.org>]

Myracle commented on code in PR #19772:
URL: https://github.com/apache/flink/pull/19772#discussion_r883388549


##########
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontend.java:
##########
@@ -1288,4 +1300,26 @@ private static CustomCommandLine loadCustomCommandLine(String className, Object.
 
         return constructor.newInstance(params);
     }
+
+    /**
+     * Get client timeout from command line via effective configuration.
+     *
+     * @param effectiveConfiguration Flink effective configuration.
+     * @return client timeout with Duration type
+     */
+    private Duration getClientTimeout(Configuration effectiveConfiguration) {
+        logAndSysout("Reading " + ClientOptions.CLIENT_TIMEOUT.key() + " from cli.");
+        return effectiveConfiguration.get(ClientOptions.CLIENT_TIMEOUT);
+    }

Review Comment:
   The clientTimeout may be read from the original config. Change the log or remove it.



##########
flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontend.java:
##########
@@ -110,10 +110,6 @@ public class CliFrontend {
 
     private final Options customCommandLineOptions;
 
-    private final Duration clientTimeout;
-
-    private final int defaultParallelism;
-

Review Comment:
   In this issue, we should only fix clientTimeout as a minor change. For defaultParallelism, it is controlled by the option -p in CLI and only used in run command.



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

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org