fsk119 commented on code in PR #21752:
URL: https://github.com/apache/flink/pull/21752#discussion_r1089917347


##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/SqlClient.java:
##########
@@ -56,38 +57,52 @@
  * <p>- In embedded mode, the SQL CLI is tightly coupled with the executor in 
a common process. This
  * allows for submitting jobs without having to start an additional component.
  *
- * <p>- In future versions: In gateway mode, the SQL CLI client connects to 
the REST API of the
- * gateway and allows for managing queries via console.
- *
- * <p>For debugging in an IDE you can execute the main method of this class 
using: "--defaults
- * /path/to/sql-client-defaults.yaml --jar 
/path/to/target/flink-sql-client-*.jar"
- *
- * <p>Make sure that the FLINK_CONF_DIR environment variable is set.
+ * <p>- In gateway mode, the SQL CLI client connects to the REST API of the 
gateway and allows for
+ * managing queries via console.
  */
 public class SqlClient {
 
     private static final Logger LOG = LoggerFactory.getLogger(SqlClient.class);
 
-    private final boolean isEmbedded;
+    private final boolean isGatewayMode;
     private final CliOptions options;
     private final Supplier<Terminal> terminalFactory;
 
     public static final String MODE_EMBEDDED = "embedded";
     public static final String MODE_GATEWAY = "gateway";
+    public static final String MODE_NONE = "";
 
-    public SqlClient(boolean isEmbedded, CliOptions options, 
Supplier<Terminal> terminalFactory) {
-        this.isEmbedded = isEmbedded;
+    public SqlClient(
+            boolean isGatewayMode, CliOptions options, Supplier<Terminal> 
terminalFactory) {
+        this.isGatewayMode = isGatewayMode;
         this.options = options;
         this.terminalFactory = terminalFactory;
     }
 
     private void start() {
-        if (isEmbedded) {
-            DefaultContext defaultContext = 
LocalContextUtils.buildDefaultContext(options);
+        if (isGatewayMode) {
+            CliOptions.GatewayCliOptions gatewayCliOptions = 
(CliOptions.GatewayCliOptions) options;
+            try (ExecutorImpl executor =
+                    new ExecutorImpl(
+                            
DefaultContextUtils.buildDefaultContext(gatewayCliOptions),
+                            gatewayCliOptions
+                                    .getGatewayAddress()
+                                    .orElseThrow(
+                                            () ->
+                                                    new SqlClientException(
+                                                            "Please specify 
the address of the SQL Gateway with command line option"
+                                                                    + " 
'-e,--endpoint <SQL Gateway address>' in the gateway mode.")))) {

Review Comment:
   It's just to allow users to specify `./sql-client.sh gateway --help`. If we 
make the option `--endpoint` required, users need to specify `./sql-client.sh 
gateway --help --endpoint 'xxxx'` to help messages. 
   
   BTW, many clients will set a default value for host address, e.g. mysql/pg. 
I think we can start a discussion about this in the future.



-- 
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-unsubscr...@flink.apache.org

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

Reply via email to