yaooqinn commented on code in PR #2336:
URL: https://github.com/apache/incubator-kyuubi/pull/2336#discussion_r849029308


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala:
##########
@@ -17,101 +17,71 @@
 
 package org.apache.kyuubi.engine.trino
 
-import java.net.URI
-import java.nio.file.Files
+import java.io.File
 import java.nio.file.Paths
+import java.util.LinkedHashSet
 
-import org.apache.kyuubi.{KYUUBI_VERSION, KyuubiSQLException, Logging, 
SCALA_COMPILE_VERSION, Utils}
+import scala.collection.JavaConverters._
+import scala.collection.mutable.ArrayBuffer
+
+import org.apache.kyuubi.{Logging, SCALA_COMPILE_VERSION}
 import org.apache.kyuubi.config.KyuubiConf
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_TRINO_CONNECTION_CATALOG
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_TRINO_CONNECTION_URL
-import org.apache.kyuubi.config.KyuubiConf.ENGINE_TRINO_MAIN_RESOURCE
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_TRINO_CONNECTION_CATALOG, 
ENGINE_TRINO_CONNECTION_URL}
+import org.apache.kyuubi.config.KyuubiReservedKeys.KYUUBI_SESSION_USER_KEY
 import org.apache.kyuubi.engine.ProcBuilder
-import org.apache.kyuubi.engine.trino.TrinoProcessBuilder._
 import org.apache.kyuubi.operation.log.OperationLog
 
 class TrinoProcessBuilder(
     override val proxyUser: String,
     override val conf: KyuubiConf,
     val extraEngineLog: Option[OperationLog] = None) extends ProcBuilder with 
Logging {
 
-  private[trino] lazy val trinoConf: Map[String, String] = {
-    assert(
-      conf.get(ENGINE_TRINO_CONNECTION_URL).isDefined,
-      throw KyuubiSQLException(
-        s"Trino server url can not be null! Please set 
${ENGINE_TRINO_CONNECTION_URL.key}"))
-    assert(
-      conf.get(ENGINE_TRINO_CONNECTION_CATALOG).isDefined,
-      throw KyuubiSQLException(
-        s"Trino default catalog can not be null!" +
-          s" Please set ${ENGINE_TRINO_CONNECTION_CATALOG.key}"))
+  override protected def module: String = "kyuubi-trino-engine"
 
-    conf.getAll.filter { case (k, v) =>
-      !k.startsWith("spark.") && !k.startsWith("hadoop.")
-    } + (USER -> proxyUser)
-  }
+  override protected def mainClass: String = 
"org.apache.kyuubi.engine.trino.TrinoSqlEngine"
 
-  override protected val executable: String = {
-    val trinoHomeOpt = env.get("TRINO_ENGINE_HOME").orElse {
-      val cwd = Utils.getCodeSourceLocation(getClass)
-        .split("kyuubi-server")
-      assert(cwd.length > 1)
-      Option(
-        Paths.get(cwd.head)
-          .resolve("externals")
-          .resolve(module)
-          .toFile)
-        .map(_.getAbsolutePath)
-    }
+  override protected def commands: Array[String] = {
+    require(
+      conf.get(ENGINE_TRINO_CONNECTION_URL).nonEmpty,
+      s"Trino server url can not be null! Please set 
${ENGINE_TRINO_CONNECTION_URL.key}")
+    require(
+      conf.get(ENGINE_TRINO_CONNECTION_CATALOG).nonEmpty,
+      s"Trino default catalog can not be null! Please set 
${ENGINE_TRINO_CONNECTION_CATALOG.key}")
+    val buffer = new ArrayBuffer[String]()
+    buffer += executable
 
-    trinoHomeOpt.map { dir =>
-      Paths.get(dir, "bin", 
TRINO_ENGINE_BINARY_FILE).toAbsolutePath.toFile.getCanonicalPath
-    }.getOrElse {
-      throw KyuubiSQLException("TRINO_ENGINE_HOME is not set! " +
-        "For more detail information on installing and configuring Trino, 
please visit " +
-        
"https://kyuubi.apache.org/docs/latest/deployment/settings.html#environments";)
-    }
-  }
+    // 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"
 
-  override protected def mainResource: Option[String] = {
-    val jarName = s"${module}_$SCALA_COMPILE_VERSION-$KYUUBI_VERSION.jar"
-    // 1. get the main resource jar for user specified config first
-    conf.get(ENGINE_TRINO_MAIN_RESOURCE).filter { userSpecified =>
-      // skip check exist if not local file.
-      val uri = new URI(userSpecified)
-      val schema = if (uri.getScheme != null) uri.getScheme else "file"
-      schema match {
-        case "file" => Files.exists(Paths.get(userSpecified))
-        case _ => true
-      }
-    }.orElse {
-      // 2. get the main resource jar from system build default
-      env.get(KyuubiConf.KYUUBI_HOME)
-        .map { Paths.get(_, "externals", "engines", "trino", "jars", jarName) }
-        .filter(Files.exists(_)).map(_.toAbsolutePath.toFile.getCanonicalPath)
-    }.orElse {
-      // 3. get the main resource from dev environment
-      Option(Paths.get("externals", module, "target", jarName))
-        .filter(Files.exists(_)).orElse {
-          Some(Paths.get("..", "externals", module, "target", jarName))
-        }.map(_.toAbsolutePath.toFile.getCanonicalPath)
+    // TODO: add Kyuubi.engineEnv.TRINO_ENGINE_MEMORY or 
kyuubi.engine.trino.memory to configure
+    // -Xmx5g
+    // java options
+    for ((k, v) <- conf.getAll) {
+      buffer += s"-D$k=$v"
     }
-  }
-
-  override protected def module: String = "kyuubi-trino-engine"
 
-  override protected def mainClass: String = 
"org.apache.kyuubi.engine.trino.TrinoSqlEngine"
+    buffer += "-cp"
+    val classpathEntries = new LinkedHashSet[String]
+    // trino engine runtime jar
+    mainResource.foreach(classpathEntries.add)
 
-  override protected def childProcEnv: Map[String, String] = conf.getEnvs +
-    ("TRINO_ENGINE_JAR" -> mainResource.get) +
-    ("TRINO_ENGINE_DYNAMIC_ARGS" ->
-      trinoConf.map { case (k, v) => s"-D$k=$v" }.mkString(" "))
+    mainResource.foreach { path =>
+      classpathEntries.add(s"$path${File.separator}*")
+      val trinoDeps = Paths.get(path).getParent
+        .resolve(s"scala-$SCALA_COMPILE_VERSION")
+        .resolve("jars")

Review Comment:
   sorry, this is just for test



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to