[incubator-kyuubi] branch master updated: [KYUUBI #2368] [Improvement] Command OptionParser for launching Trino Backend Engine

[yao]

This is an automated email from the ASF dual-hosted git repository.

yao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git


The following commit(s) were added to refs/heads/master by this push:
     new 4f4960cf4 [KYUUBI #2368] [Improvement] Command OptionParser for 
launching Trino Backend Engine
4f4960cf4 is described below

commit 4f4960cf4d1769e3970f3d5bbe548aa05b8d00b5
Author: Min Zhao <zhaomin1...@163.com>
AuthorDate: Tue Apr 19 15:55:57 2022 +0800

    [KYUUBI #2368] [Improvement] Command OptionParser for launching Trino 
Backend Engine
    
    ### _Why are the changes needed?_
    
    Command OptionParser for launching Trino Backend Engine
    ### _How was this patch tested?_
    - [ ] Add some test cases that check the changes thoroughly including 
negative and positive cases if possible
    
    - [ ] Add screenshots for manual tests if appropriate
    
    - [ ] [Run 
test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests)
 locally before make a pull request
    
    Closes #2417 from zhaomin1423/trino_option_parser.
    
    Closes #2368
    
    c644c791 [Min Zhao] [KYUUBI #2368] [Improvement] Command OptionParser for 
launching Trino Backend Engine
    
    Authored-by: Min Zhao <zhaomin1...@163.com>
    Signed-off-by: Kent Yao <y...@apache.org>
---
 .../kyuubi/engine/trino/TrinoSqlEngine.scala       |  3 ++-
 .../kyuubi/engine/trino/TrinoProcessBuilder.scala  | 23 +++++++++++-----------
 .../engine/trino/TrinoProcessBuilderSuite.scala    |  6 +++++-
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git 
a/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
 
b/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
index d05bc641c..1a2534fb7 100644
--- 
a/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
+++ 
b/externals/kyuubi-trino-engine/src/main/scala/org/apache/kyuubi/engine/trino/TrinoSqlEngine.scala
@@ -19,7 +19,7 @@ package org.apache.kyuubi.engine.trino
 
 import java.util.concurrent.CountDownLatch
 
-import org.apache.kyuubi.Logging
+import org.apache.kyuubi.{Logging, Utils}
 import org.apache.kyuubi.Utils.TRINO_ENGINE_SHUTDOWN_PRIORITY
 import org.apache.kyuubi.Utils.addShutdownHook
 import org.apache.kyuubi.config.KyuubiConf
@@ -72,6 +72,7 @@ object TrinoSqlEngine extends Logging {
     SignalRegister.registerLogger(logger)
 
     try {
+      Utils.fromCommandLineArgs(args, kyuubiConf)
       kyuubiConf.setIfMissing(KyuubiConf.FRONTEND_THRIFT_BINARY_BIND_PORT, 0)
       kyuubiConf.setIfMissing(HA_ZK_CONN_RETRY_POLICY, 
RetryPolicies.N_TIME.toString)
 
diff --git 
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
 
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
index cb33f8f40..70cecb8f5 100644
--- 
a/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
+++ 
b/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala
@@ -50,23 +50,12 @@ class TrinoProcessBuilder(
     val buffer = new ArrayBuffer[String]()
     buffer += executable
 
-    // TODO: How shall we deal with proxyUser,
-    // user.name
-    // kyuubi.session.user
-    // or just leave it, because we can handle it at operation layer
-    buffer += s"-D$KYUUBI_SESSION_USER_KEY=$proxyUser"
-
     val memory = conf.get(ENGINE_TRINO_MEMORY)
     buffer += s"-Xmx$memory"
     val javaOptions = conf.get(ENGINE_TRINO_JAVA_OPTIONS)
     if (javaOptions.isDefined) {
       buffer += javaOptions.get
     }
-    // -Xmx5g
-    // java options
-    for ((k, v) <- conf.getAll) {
-      buffer += s"-D$k=$v"
-    }
 
     buffer += "-cp"
     val classpathEntries = new LinkedHashSet[String]
@@ -92,6 +81,18 @@ class TrinoProcessBuilder(
 
     buffer += classpathEntries.asScala.mkString(File.pathSeparator)
     buffer += mainClass
+
+    // TODO: How shall we deal with proxyUser,
+    // user.name
+    // kyuubi.session.user
+    // or just leave it, because we can handle it at operation layer
+    buffer += "--conf"
+    buffer += s"$KYUUBI_SESSION_USER_KEY=$proxyUser"
+
+    for ((k, v) <- conf.getAll) {
+      buffer += "--conf"
+      buffer += s"$k=$v"
+    }
     buffer.toArray
   }
 
diff --git 
a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilderSuite.scala
 
b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilderSuite.scala
index 7afc1ed06..2c37c41bc 100644
--- 
a/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilderSuite.scala
+++ 
b/kyuubi-server/src/test/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilderSuite.scala
@@ -20,6 +20,7 @@ package org.apache.kyuubi.engine.trino
 import org.apache.kyuubi.KyuubiFunSuite
 import org.apache.kyuubi.config.KyuubiConf
 import org.apache.kyuubi.config.KyuubiConf._
+import org.apache.kyuubi.config.KyuubiReservedKeys.KYUUBI_SESSION_USER_KEY
 
 class TrinoProcessBuilderSuite extends KyuubiFunSuite {
 
@@ -30,7 +31,10 @@ class TrinoProcessBuilderSuite extends KyuubiFunSuite {
     val builder = new TrinoProcessBuilder("kyuubi", conf)
     val commands = builder.toString.split("\n")
     assert(commands.head.endsWith("java"))
-    
assert(commands.contains(s"-D${ENGINE_TRINO_CONNECTION_URL.key}=dummy_url"))
+    
assert(builder.toString.contains(s"--conf\n${KYUUBI_SESSION_USER_KEY}=kyuubi"))
+    
assert(builder.toString.contains(s"--conf\n${ENGINE_TRINO_CONNECTION_URL.key}=dummy_url"))
+    assert(builder.toString.contains(
+      s"--conf\n${ENGINE_TRINO_CONNECTION_CATALOG.key}=dummy_catalog"))
   }
 
   test("capture error from trino process builder") {