You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/04/19 07:51:45 UTC

[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #2418: [KYUUBI #2346] [Improvement] Simplify FlinkProcessBuilder with java executable

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


##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/flink/FlinkProcessBuilder.scala:
##########
@@ -40,44 +42,74 @@ class FlinkProcessBuilder(
     val extraEngineLog: Option[OperationLog] = None)
   extends ProcBuilder with Logging {
 
-  override protected def executable: String = {
-    val flinkEngineHomeOpt = env.get("FLINK_ENGINE_HOME").orElse {
-      val cwd = Utils.getCodeSourceLocation(getClass)
-        .split("kyuubi-server")
-      assert(cwd.length > 1)
-      Option(
-        Paths.get(cwd.head)
-          .resolve("externals")
-          .resolve("kyuubi-flink-sql-engine")
-          .toFile)
-        .map(_.getAbsolutePath)
-    }
-
-    flinkEngineHomeOpt.map { dir =>
-      Paths.get(dir, "bin", FLINK_ENGINE_BINARY_FILE).toAbsolutePath.toFile.getCanonicalPath
-    } getOrElse {
-      throw KyuubiSQLException("FLINK_ENGINE_HOME is not set! " +
-        "For more detail information on installing and configuring Flink, please visit " +
-        "https://kyuubi.apache.org/docs/latest/deployment/settings.html#environments")
-    }
-  }
-
   override protected def module: String = "kyuubi-flink-sql-engine"
 
   override protected def mainClass: String = "org.apache.kyuubi.engine.flink.FlinkSQLEngine"
 
   override protected def childProcEnv: Map[String, String] = conf.getEnvs +
     ("FLINK_HOME" -> FLINK_HOME) +
     ("FLINK_CONF_DIR" -> s"$FLINK_HOME/conf") +
-    ("FLINK_SQL_ENGINE_JAR" -> mainResource.get) +
-    ("FLINK_SQL_ENGINE_DYNAMIC_ARGS" ->
-      conf.getAll.filter { case (k, _) =>
-        k.startsWith("kyuubi.") || k.startsWith("flink.") ||
-          k.startsWith("hadoop.") || k.startsWith("yarn.")
-      }.map { case (k, v) => s"-D$k=$v" }.mkString(" "))
+    ("FLINK_SQL_ENGINE_JAR" -> mainResource.get)

Review Comment:
   we can remove these



-- 
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: notifications-unsubscribe@kyuubi.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org