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/06/07 11:48:10 UTC

[GitHub] [incubator-kyuubi] lightning-L opened a new pull request, #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

lightning-L opened a new pull request, #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823

   <!--
   Thanks for sending a pull request!
   
   Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
     2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
   -->
   
   ### _Why are the changes needed?_
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you add a feature, you can talk about the use case of it.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   
   
   ### _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
   


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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891170975


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -600,6 +600,28 @@ object KyuubiConf {
       .version("1.4.0")
       .fallbackConf(FRONTEND_WORKER_KEEPALIVE_TIME)
 
+  val REST_CLIENT_BASE_URL: OptionalConfigEntry[String] =
+    buildConf("kyuubi.rest.base.url")

Review Comment:
   Move these config items to CtlConf, please refer `HighAvailabilityConf`.



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891228706


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.create
+
+import java.util.{ArrayList, HashMap}
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest}
+import org.apache.kyuubi.client.util.JsonUtil
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class CreateBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  private var map: HashMap[String, Object] = null
+
+  /** Ensure that required fields exists. Call this only once all defaults are loaded. */
+  override def validateArguments(): Unit = {
+    map = readConfig()
+  }
+
+  override def run(): Unit = {
+
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, map, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    val request = map.get("request").asInstanceOf[HashMap[String, Object]]
+    val batchRequest = new BatchRequest(
+      map.get("batchType").asInstanceOf[String],
+      request.get("resource").asInstanceOf[String],
+      request.get("className").asInstanceOf[String],
+      request.get("name").asInstanceOf[String],
+      request.get("config").asInstanceOf[HashMap[String, String]],
+      request.get("args").asInstanceOf[ArrayList[String]])
+
+    val batch: Batch = batchRestApi.createBatch(batchRequest)
+    info(JsonUtil.toJson(batch))
+
+    kyuubiRestClient.close()

Review Comment:
   seems that we can define a method named `withKyuubiRestClient` in RestClientFactory.
   
   It will close the kyuubiRestClient in finally block.  likes:
   
   ```
     private def withInternalAuthHeader[T](user: String)(f: String => T): T = {
       val encodedAuthorization = new String(
         Base64.getEncoder.encode(s"$user:${InternalSecurityAccessor.get().issueToken()}".getBytes()),
         "UTF-8")
       val internalAuthHeader = s"${AuthSchemes.KYUUBI_INTERNAL.toString} $encodedAuthorization"
       try {
         KyuubiRestClient.setThreadLocalAuthHeader(internalAuthHeader)
         f(internalAuthHeader)
       } finally {
         KyuubiRestClient.clearThreadLocalAuthHeader()
       }
     }
   ```



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891178162


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ClientFactory.scala:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl
+
+import java.util.HashMap
+
+import scala.reflect.runtime.{universe => ru}
+import scala.reflect.runtime.universe._
+
+import org.apache.commons.lang3.StringUtils
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.client.KyuubiRestClient
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+
+object ClientFactory {

Review Comment:
   RestClientFactory?



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CliConfig.scala:
##########
@@ -41,7 +44,25 @@ case class CommonOpts(
     host: String = null,
     port: String = null,
     version: String = null,
-    verbose: Boolean = false)
+    verbose: Boolean = false,
+    hostUrl: String = null,
+    authSchema: String = null,
+    username: String = null,
+    password: String = null,
+    spnegoHost: String = null)
+
+case class CreateOpts(
+    filename: String = null)
+
+case class LogOpts(
+    forward: Boolean = false)
+
+case class BatchOpts(
+    batchId: String = null,
+    batchType: String = null,

Review Comment:
   now there are more filter conditions for listBatches.
   
   https://github.com/apache/incubator-kyuubi/blob/9ee4a9e37f2eaf25acdfd88e8f886b7b43691981/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/BatchRestApi.java#L51-L60



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ClientFactory.scala:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl
+
+import java.util.HashMap
+
+import scala.reflect.runtime.{universe => ru}
+import scala.reflect.runtime.universe._
+
+import org.apache.commons.lang3.StringUtils
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.client.KyuubiRestClient
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+
+object ClientFactory {
+
+  private[ctl] def getKyuubiRestClient(
+      cliArgs: CliConfig,
+      map: HashMap[String, Object],
+      conf: KyuubiConf): KyuubiRestClient = {
+    val version = getApiVersion(map)
+    val hostUrl = getRestConfig("hostUrl", conf.get(REST_CLIENT_BASE_URL).get, cliArgs, map)
+    val authSchema = getRestConfig("authSchema", conf.get(REST_CLIENT_AUTH_SCHEMA), cliArgs, map)
+
+    var kyuubiRestClient: KyuubiRestClient = null
+    authSchema match {
+      case "basic" =>
+        val username = getRestConfig("username", null, cliArgs, map)
+        val password = cliArgs.commonOpts.password
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.BASIC)
+          .username(username)
+          .password(password)
+          .build()
+      case "spnego" =>
+        val spnegoHost =
+          getRestConfig("spnegoHost", conf.get(REST_CLIENT_SPNEGO_HOST).get, cliArgs, map)
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.SPNEGO)
+          .spnegoHost(spnegoHost)
+          .build()
+      case _ => throw new KyuubiException(s"Unsupported auth schema: $authSchema")
+    }
+    kyuubiRestClient
+  }
+
+  private def getApiVersion(map: HashMap[String, Object]): String = {
+    var version: String = "V1"
+    if (map != null) {
+      val configuredVersion = map.get("apiVersion").asInstanceOf[String].toUpperCase
+      if (StringUtils.isNotBlank(configuredVersion)) {
+        version = configuredVersion
+      }
+    }
+    version
+  }
+
+  private def getRestConfig(
+      key: String,
+      defaultValue: String,
+      cliArgs: CliConfig,
+      map: HashMap[String, Object]): String = {
+    var configValue: String = null
+    // get value from command line
+    val commonOpts = cliArgs.commonOpts
+    val mirror = ru.runtimeMirror(getClass.getClassLoader)
+    val instanceMirror = mirror.reflect(commonOpts)
+    val field = typeOf[CommonOpts].decl(TermName(key)).asTerm.accessed.asTerm
+    val fieldMirror = instanceMirror.reflectField(field)
+    configValue = fieldMirror.get.asInstanceOf[String]

Review Comment:
   why we need to use reflection?
   



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ClientFactory.scala:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl
+
+import java.util.HashMap
+
+import scala.reflect.runtime.{universe => ru}
+import scala.reflect.runtime.universe._
+
+import org.apache.commons.lang3.StringUtils
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.client.KyuubiRestClient
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+
+object ClientFactory {
+
+  private[ctl] def getKyuubiRestClient(
+      cliArgs: CliConfig,
+      map: HashMap[String, Object],
+      conf: KyuubiConf): KyuubiRestClient = {
+    val version = getApiVersion(map)
+    val hostUrl = getRestConfig("hostUrl", conf.get(REST_CLIENT_BASE_URL).get, cliArgs, map)
+    val authSchema = getRestConfig("authSchema", conf.get(REST_CLIENT_AUTH_SCHEMA), cliArgs, map)
+
+    var kyuubiRestClient: KyuubiRestClient = null
+    authSchema match {
+      case "basic" =>
+        val username = getRestConfig("username", null, cliArgs, map)
+        val password = cliArgs.commonOpts.password
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.BASIC)
+          .username(username)
+          .password(password)
+          .build()
+      case "spnego" =>
+        val spnegoHost =
+          getRestConfig("spnegoHost", conf.get(REST_CLIENT_SPNEGO_HOST).get, cliArgs, map)
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.SPNEGO)
+          .spnegoHost(spnegoHost)
+          .build()
+      case _ => throw new KyuubiException(s"Unsupported auth schema: $authSchema")
+    }
+    kyuubiRestClient
+  }
+
+  private def getApiVersion(map: HashMap[String, Object]): String = {
+    var version: String = "V1"
+    if (map != null) {
+      val configuredVersion = map.get("apiVersion").asInstanceOf[String].toUpperCase
+      if (StringUtils.isNotBlank(configuredVersion)) {
+        version = configuredVersion
+      }
+    }
+    version
+  }
+
+  private def getRestConfig(
+      key: String,
+      defaultValue: String,
+      cliArgs: CliConfig,

Review Comment:
   cliConfig?



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ClientFactory.scala:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl
+
+import java.util.HashMap
+
+import scala.reflect.runtime.{universe => ru}
+import scala.reflect.runtime.universe._
+
+import org.apache.commons.lang3.StringUtils
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.client.KyuubiRestClient
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+
+object ClientFactory {
+
+  private[ctl] def getKyuubiRestClient(
+      cliArgs: CliConfig,
+      map: HashMap[String, Object],
+      conf: KyuubiConf): KyuubiRestClient = {
+    val version = getApiVersion(map)
+    val hostUrl = getRestConfig("hostUrl", conf.get(REST_CLIENT_BASE_URL).get, cliArgs, map)
+    val authSchema = getRestConfig("authSchema", conf.get(REST_CLIENT_AUTH_SCHEMA), cliArgs, map)
+
+    var kyuubiRestClient: KyuubiRestClient = null
+    authSchema match {
+      case "basic" =>
+        val username = getRestConfig("username", null, cliArgs, map)
+        val password = cliArgs.commonOpts.password
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.BASIC)
+          .username(username)
+          .password(password)
+          .build()
+      case "spnego" =>
+        val spnegoHost =
+          getRestConfig("spnegoHost", conf.get(REST_CLIENT_SPNEGO_HOST).get, cliArgs, map)
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.SPNEGO)
+          .spnegoHost(spnegoHost)
+          .build()
+      case _ => throw new KyuubiException(s"Unsupported auth schema: $authSchema")
+    }
+    kyuubiRestClient
+  }
+
+  private def getApiVersion(map: HashMap[String, Object]): String = {
+    var version: String = "V1"
+    if (map != null) {
+      val configuredVersion = map.get("apiVersion").asInstanceOf[String].toUpperCase
+      if (StringUtils.isNotBlank(configuredVersion)) {
+        version = configuredVersion
+      }
+    }
+    version
+  }
+
+  private def getRestConfig(
+      key: String,
+      defaultValue: String,
+      cliArgs: CliConfig,
+      map: HashMap[String, Object]): String = {
+    var configValue: String = null
+    // get value from command line
+    val commonOpts = cliArgs.commonOpts
+    val mirror = ru.runtimeMirror(getClass.getClassLoader)
+    val instanceMirror = mirror.reflect(commonOpts)
+    val field = typeOf[CommonOpts].decl(TermName(key)).asTerm.accessed.asTerm
+    val fieldMirror = instanceMirror.reflectField(field)
+    configValue = fieldMirror.get.asInstanceOf[String]

Review Comment:
   I see. FYI:
   
   ```
   scala> case class C1(v1 :String, v2 :String)
   class C1
   
   scala> val c1Instance = C1("this is v1", "this is v2")
   val c1Instance: C1 = C1(this is v1,this is v2)
   
   scala> val keyNames =  c1Instance.productElementNames.toSeq
   val keyNames: Seq[String] = List(v1, v2)
   
   scala> val keyIndex = keyNames.zipWithIndex.find(_._1.equals("v1")).unzip._2
   val keyIndex: Option[Int] = Some(0)
   
   scala> val value = keyIndex.map(c1Instance.productElement)
   val value: Option[Any] = Some(this is v1)
   
   scala> 
   
   ```



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891201015


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -32,9 +32,12 @@ object CommandLine {
       get(builder),
       delete(builder),
       list(builder),
+      log(builder),
+      submit(builder),
       checkConfig(f => {
-        if (f.action == null) failure("Must specify action command: [create|get|delete|list].")
-        else success
+        if (f.action == null) {
+          failure("Must specify action command: [create|get|delete|list|log|submit].")
+        } else success

Review Comment:
   ```
   else {
   success
   }
   ```



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r892527748


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ClientFactory.scala:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl
+
+import java.util.HashMap
+
+import scala.reflect.runtime.{universe => ru}
+import scala.reflect.runtime.universe._
+
+import org.apache.commons.lang3.StringUtils
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.client.KyuubiRestClient
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+
+object ClientFactory {
+
+  private[ctl] def getKyuubiRestClient(
+      cliArgs: CliConfig,
+      map: HashMap[String, Object],
+      conf: KyuubiConf): KyuubiRestClient = {
+    val version = getApiVersion(map)
+    val hostUrl = getRestConfig("hostUrl", conf.get(REST_CLIENT_BASE_URL).get, cliArgs, map)
+    val authSchema = getRestConfig("authSchema", conf.get(REST_CLIENT_AUTH_SCHEMA), cliArgs, map)
+
+    var kyuubiRestClient: KyuubiRestClient = null
+    authSchema match {
+      case "basic" =>
+        val username = getRestConfig("username", null, cliArgs, map)
+        val password = cliArgs.commonOpts.password
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.BASIC)
+          .username(username)
+          .password(password)
+          .build()
+      case "spnego" =>
+        val spnegoHost =
+          getRestConfig("spnegoHost", conf.get(REST_CLIENT_SPNEGO_HOST).get, cliArgs, map)
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.SPNEGO)
+          .spnegoHost(spnegoHost)
+          .build()
+      case _ => throw new KyuubiException(s"Unsupported auth schema: $authSchema")
+    }
+    kyuubiRestClient
+  }
+
+  private def getApiVersion(map: HashMap[String, Object]): String = {
+    var version: String = "V1"
+    if (map != null) {
+      val configuredVersion = map.get("apiVersion").asInstanceOf[String].toUpperCase
+      if (StringUtils.isNotBlank(configuredVersion)) {
+        version = configuredVersion
+      }
+    }
+    version
+  }
+
+  private def getRestConfig(
+      key: String,
+      defaultValue: String,
+      cliArgs: CliConfig,
+      map: HashMap[String, Object]): String = {
+    var configValue: String = null
+    // get value from command line
+    val commonOpts = cliArgs.commonOpts
+    val mirror = ru.runtimeMirror(getClass.getClassLoader)
+    val instanceMirror = mirror.reflect(commonOpts)
+    val field = typeOf[CommonOpts].decl(TermName(key)).asTerm.accessed.asTerm
+    val fieldMirror = instanceMirror.reflectField(field)
+    configValue = fieldMirror.get.asInstanceOf[String]

Review Comment:
   seems the `productElementNames` method is not available in scala-2.12.
   
   I think you can continue to use scala `match`, because there are only several options in CommonOpts.



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895028197


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CliConfig.scala:
##########
@@ -41,7 +44,29 @@ case class CommonOpts(
     host: String = null,
     port: String = null,
     version: String = null,
-    verbose: Boolean = false)
+    verbose: Boolean = false,
+    hostUrl: String = null,
+    authSchema: String = null,
+    username: String = null,
+    password: String = null,
+    spnegoHost: String = null)
+
+case class CreateOpts(
+    filename: String = null)
+
+case class LogOpts(
+    forward: Boolean = false)

Review Comment:
   ditto



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/Helper.scala:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.util
+
+import java.io.{BufferedReader, File, FileInputStream, InputStreamReader}
+import java.nio.charset.StandardCharsets
+import java.util.HashMap
+
+import org.yaml.snakeyaml.Yaml
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_SHARE_LEVEL, ENGINE_SHARE_LEVEL_SUBDOMAIN, ENGINE_TYPE}
+import org.apache.kyuubi.ctl.{CliConfig, ControlObject}
+import org.apache.kyuubi.ha.client.{DiscoveryClient, DiscoveryPaths, ServiceNodeInfo}
+import org.apache.kyuubi.ha.client.DiscoveryClientProvider.withDiscoveryClient
+
+object Helper {
+
+  private[ctl] def getZkNamespace(conf: KyuubiConf, cliConfig: CliConfig): String = {
+    cliConfig.resource match {
+      case ControlObject.SERVER =>
+        DiscoveryPaths.makePath(null, cliConfig.commonOpts.namespace)
+      case ControlObject.ENGINE =>
+        val engineType = Some(cliConfig.engineOpts.engineType)
+          .filter(_ != null).filter(_.nonEmpty)
+          .getOrElse(conf.get(ENGINE_TYPE))
+        val engineSubdomain = Some(cliConfig.engineOpts.engineSubdomain)
+          .filter(_ != null).filter(_.nonEmpty)
+          .getOrElse(conf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+        val engineShareLevel = Some(cliConfig.engineOpts.engineShareLevel)
+          .filter(_ != null).filter(_.nonEmpty)
+          .getOrElse(conf.get(ENGINE_SHARE_LEVEL))
+        // The path of the engine defined in zookeeper comes from
+        // org.apache.kyuubi.engine.EngineRef#engineSpace
+        DiscoveryPaths.makePath(
+          s"${cliConfig.commonOpts.namespace}_" +
+            s"${cliConfig.commonOpts.version}_" +
+            s"${engineShareLevel}_${engineType}",
+          cliConfig.engineOpts.user,
+          Array(engineSubdomain))
+    }
+  }
+
+  private[ctl] def getServiceNodes(
+      discoveryClient: DiscoveryClient,
+      znodeRoot: String,
+      hostPortOpt: Option[(String, Int)]): Seq[ServiceNodeInfo] = {
+    val serviceNodes = discoveryClient.getServiceNodesInfo(znodeRoot)
+    hostPortOpt match {
+      case Some((host, port)) => serviceNodes.filter { sn =>
+          sn.host == host && sn.port == port
+        }
+      case _ => serviceNodes
+    }
+  }
+
+  /**
+   * List Kyuubi server nodes info.
+   */
+  private[ctl] def listZkServerNodes(
+      conf: KyuubiConf,
+      cliConfig: CliConfig,
+      filterHostPort: Boolean): Seq[ServiceNodeInfo] = {
+    var nodes = Seq.empty[ServiceNodeInfo]
+    withDiscoveryClient(conf) { discoveryClient =>
+      val znodeRoot = Helper.getZkNamespace(conf, cliConfig)
+      val hostPortOpt =
+        if (filterHostPort) {
+          Some((cliConfig.commonOpts.host, cliConfig.commonOpts.port.toInt))
+        } else None
+      nodes = Helper.getServiceNodes(discoveryClient, znodeRoot, hostPortOpt)
+    }
+    nodes
+  }
+
+  private[ctl] def loadYamlIntoMap(cliConfig: CliConfig): HashMap[String, Object] = {

Review Comment:
   the return type could be `JMap[String, Object]`.
   
   ```
   import java.util.{Map => JMap}
   ```



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -62,16 +68,36 @@ object CommandLine {
           " change it if the active service is running in another."),
       opt[Unit]('b', "verbose")
         .action((_, c) => c.copy(commonOpts = c.commonOpts.copy(verbose = true)))
-        .text("Print additional debug output."))
+        .text("Print additional debug output."),
+      opt[String]("hostUrl")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(hostUrl = v)))
+        .text("Host url for rest api."),
+      opt[String]("authSchema")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(authSchema = v)))
+        .text("Auth schema for rest api."),
+      opt[String]("username")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(username = v)))
+        .text("Username for basic authentication."),
+      opt[String]("password")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(password = v)))
+        .text("Password for rest api."),
+      opt[String]("spnegoHost")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(spnegoHost = v)))
+        .text("Spnego host for rest api."))
   }
 
   private def create(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
     import builder._
     OParser.sequence(
       note(""),
       cmd("create")
-        .action((_, c) => c.copy(action = ServiceControlAction.CREATE))
+        .text("\tCreate a resource.")
+        .action((_, c) => c.copy(action = ControlAction.CREATE))
         .children(
+          opt[String]('f', "filename")
+            .action((v, c) => c.copy(createOpts = c.createOpts.copy(filename = v)))
+            .text("Filename to use to create the resource"),

Review Comment:
   Seems some text added `\t`, not sure whether here also need it.



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.create
+
+import java.util.{ArrayList, HashMap}
+
+import org.apache.kyuubi.client.BatchRestApi
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest}
+import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
+import org.apache.kyuubi.ctl.cmd.Command
+import org.apache.kyuubi.ctl.util.{Helper, Render, Validator}
+
+class CreateBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  def validate(): Unit = {
+    Validator.validateFilename(normalizedCliConfig)
+  }
+
+  def run(): Unit = {
+    val map: HashMap[String, Object] = Helper.loadYamlIntoMap(normalizedCliConfig)

Review Comment:
   ```  
     val map = Helper.loadYamlIntoMap(normalizedCliConfig)
   ```



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/CommandLineUtils.scala:
##########
@@ -41,6 +43,22 @@ private[kyuubi] trait CommandLineLoggingUtils {
   // scalastyle:on println
 }
 
+private[kyuubi] object DateTimeUtil {
+
+  def dateStringToMillis(date: String, format: String): Long = {
+    if (date == null) return 0
+    val localDateTime = LocalDateTime.parse(date, DateTimeFormatter.ofPattern(format))
+    localDateTime.atZone(ZoneId.systemDefault).toInstant.toEpochMilli
+  }
+
+  def millisToDateString(millis: Long, format: String): String = {
+    val formatter = DateTimeFormatter.ofPattern(format)
+    val date: LocalDateTime =
+      Instant.ofEpochMilli(millis).atZone(ZoneId.systemDefault()).toLocalDateTime()
+    formatter.format(date)
+  }
+}
+
 /** Refer the showString method of org.apache.spark.sql.Dataset */
 private[kyuubi] object Tabulator {

Review Comment:
   could you help move  `Tabulator` to `Tabulator.scala`?



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ControlCliArguments.scala:
##########
@@ -57,18 +59,28 @@ class ControlCliArguments(args: Seq[String], env: Map[String, String] = sys.env)
   }
 
   private def getCommand(cliArgs: CliConfig): Command = {
-    cliArgs.action match {
-      case ServiceControlAction.CREATE => new CreateCommand(cliArgs)
-      case ServiceControlAction.GET => new GetCommand(cliArgs)
-      case ServiceControlAction.DELETE => new DeleteCommand(cliArgs)
-      case ServiceControlAction.LIST => new ListCommand(cliArgs)
-      case _ => null
-    }
+    val operationName = cliArgs.action.toString.toLowerCase
+    val resourceName = cliArgs.service.toString.toLowerCase
+
+    val packageName = operationName
+    val commandName = s"${operationName.capitalize}${resourceName.capitalize}"
+    val className = s"org.apache.kyuubi.ctl.cmd.${packageName}.${commandName}Command"
+
+    val mirror: ru.Mirror = ru.runtimeMirror(getClass.getClassLoader)
+    val classSymbol: ru.ClassSymbol = mirror.staticClass(className)
+
+    val consMethodSymbol = classSymbol.primaryConstructor.asMethod
+
+    val classMirror = mirror.reflectClass(classSymbol)
+    val consMethodMirror = classMirror.reflectConstructor(consMethodSymbol)
+
+    val result = consMethodMirror.apply(cliArgs).asInstanceOf[Command]
+    result

Review Comment:
   we can use match and aggregate the match cases by `resource` type.



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/submit/SubmitBatchCommand.scala:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.submit
+
+import java.util.{ArrayList, HashMap}
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.BatchRestApi
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest, OperationLog}
+import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
+import org.apache.kyuubi.ctl.cmd.Command
+import org.apache.kyuubi.ctl.util.{Helper, Validator}
+
+class SubmitBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  def validate(): Unit = {
+    Validator.validateFilename(normalizedCliConfig)
+  }
+
+  def run(): Unit = {
+    val map: HashMap[String, Object] = Helper.loadYamlIntoMap(normalizedCliConfig)
+
+    withKyuubiRestClient(normalizedCliConfig, map, conf) { kyuubiRestClient =>
+      val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+      val request = map.get("request").asInstanceOf[HashMap[String, Object]]
+      val batchRequest = new BatchRequest(
+        map.get("batchType").asInstanceOf[String],
+        request.get("resource").asInstanceOf[String],
+        request.get("className").asInstanceOf[String],
+        request.get("name").asInstanceOf[String],
+        request.get("configs").asInstanceOf[HashMap[String, String]],
+        request.get("args").asInstanceOf[ArrayList[String]])
+
+      var batch: Batch = batchRestApi.createBatch(batchRequest)
+      val batchId = batch.getId
+      var log: OperationLog = null
+      var done = false
+      var from = 0
+      val size = 20
+      while (!done) {
+        log = batchRestApi.getBatchLocalLog(
+          batchId,
+          from,
+          size)
+        from += log.getLogRowSet.size
+        log.getLogRowSet.asScala.foreach(x => info(x))
+
+        batch = batchRestApi.getBatchById(batchId)
+        if (log.getLogRowSet.size() == 0 && batch.getState() != "PENDING"
+          && batch.getState() != "RUNNING") {
+          done = true
+        }
+      }

Review Comment:
   we can do some enhancements in another pr for follow up:
   
   1. get the kyuubi instance that created this batch
   2. always call the kyuubi instance that created this batch to get local logs to prevent request redirection across kyuubi instances
   3. if the request(get state) to the kyuubi instance that created this batch failed, call the base url(might be a VIP for load balance and HA)



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CliConfig.scala:
##########
@@ -41,7 +44,29 @@ case class CommonOpts(
     host: String = null,
     port: String = null,
     version: String = null,
-    verbose: Boolean = false)
+    verbose: Boolean = false,
+    hostUrl: String = null,
+    authSchema: String = null,
+    username: String = null,
+    password: String = null,
+    spnegoHost: String = null)
+
+case class CreateOpts(
+    filename: String = null)

Review Comment:
   nit: new line is not needed if only has one filed



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/Helper.scala:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.util
+
+import java.io.{BufferedReader, File, FileInputStream, InputStreamReader}
+import java.nio.charset.StandardCharsets
+import java.util.HashMap
+
+import org.yaml.snakeyaml.Yaml
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_SHARE_LEVEL, ENGINE_SHARE_LEVEL_SUBDOMAIN, ENGINE_TYPE}
+import org.apache.kyuubi.ctl.{CliConfig, ControlObject}
+import org.apache.kyuubi.ha.client.{DiscoveryClient, DiscoveryPaths, ServiceNodeInfo}
+import org.apache.kyuubi.ha.client.DiscoveryClientProvider.withDiscoveryClient
+
+object Helper {
+
+  private[ctl] def getZkNamespace(conf: KyuubiConf, cliConfig: CliConfig): String = {
+    cliConfig.resource match {
+      case ControlObject.SERVER =>
+        DiscoveryPaths.makePath(null, cliConfig.commonOpts.namespace)
+      case ControlObject.ENGINE =>
+        val engineType = Some(cliConfig.engineOpts.engineType)
+          .filter(_ != null).filter(_.nonEmpty)
+          .getOrElse(conf.get(ENGINE_TYPE))
+        val engineSubdomain = Some(cliConfig.engineOpts.engineSubdomain)
+          .filter(_ != null).filter(_.nonEmpty)
+          .getOrElse(conf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN).getOrElse("default"))
+        val engineShareLevel = Some(cliConfig.engineOpts.engineShareLevel)
+          .filter(_ != null).filter(_.nonEmpty)
+          .getOrElse(conf.get(ENGINE_SHARE_LEVEL))
+        // The path of the engine defined in zookeeper comes from
+        // org.apache.kyuubi.engine.EngineRef#engineSpace
+        DiscoveryPaths.makePath(
+          s"${cliConfig.commonOpts.namespace}_" +
+            s"${cliConfig.commonOpts.version}_" +
+            s"${engineShareLevel}_${engineType}",
+          cliConfig.engineOpts.user,
+          Array(engineSubdomain))
+    }
+  }
+
+  private[ctl] def getServiceNodes(
+      discoveryClient: DiscoveryClient,
+      znodeRoot: String,
+      hostPortOpt: Option[(String, Int)]): Seq[ServiceNodeInfo] = {
+    val serviceNodes = discoveryClient.getServiceNodesInfo(znodeRoot)
+    hostPortOpt match {
+      case Some((host, port)) => serviceNodes.filter { sn =>
+          sn.host == host && sn.port == port
+        }
+      case _ => serviceNodes
+    }
+  }
+
+  /**
+   * List Kyuubi server nodes info.
+   */
+  private[ctl] def listZkServerNodes(
+      conf: KyuubiConf,
+      cliConfig: CliConfig,
+      filterHostPort: Boolean): Seq[ServiceNodeInfo] = {
+    var nodes = Seq.empty[ServiceNodeInfo]
+    withDiscoveryClient(conf) { discoveryClient =>
+      val znodeRoot = Helper.getZkNamespace(conf, cliConfig)
+      val hostPortOpt =
+        if (filterHostPort) {
+          Some((cliConfig.commonOpts.host, cliConfig.commonOpts.port.toInt))
+        } else None
+      nodes = Helper.getServiceNodes(discoveryClient, znodeRoot, hostPortOpt)
+    }
+    nodes
+  }
+
+  private[ctl] def loadYamlIntoMap(cliConfig: CliConfig): HashMap[String, Object] = {

Review Comment:
   loadYamlAsMap?



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/CommandLineUtils.scala:
##########
@@ -41,6 +43,22 @@ private[kyuubi] trait CommandLineLoggingUtils {
   // scalastyle:on println
 }
 
+private[kyuubi] object DateTimeUtil {

Review Comment:
   you can add a new file named `DateTimeUtil.scala`.



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.log
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.BatchRestApi
+import org.apache.kyuubi.client.api.v1.dto.{Batch, OperationLog}
+import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
+import org.apache.kyuubi.ctl.cmd.Command
+
+class LogBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  def validate(): Unit = {
+    if (normalizedCliConfig.batchOpts.batchId == null) {
+      fail("Must specify batchId for log batch command.")
+    }
+  }
+
+  def run(): Unit = {
+    withKyuubiRestClient(normalizedCliConfig, null, conf) { kyuubiRestClient =>
+      val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+      val batchId = normalizedCliConfig.batchOpts.batchId
+      var from = normalizedCliConfig.batchOpts.from
+      val size = normalizedCliConfig.batchOpts.size
+      var log: OperationLog = batchRestApi.getBatchLocalLog(
+        batchId,
+        from,
+        size)
+      log.getLogRowSet.asScala.foreach(x => info(x))
+
+      var done = false
+      var batch: Batch = null
+      from = if (from < 0) log.getLogRowSet.size else from + log.getLogRowSet.size
+      if (normalizedCliConfig.logOpts.forward) {
+        while (!done) {
+          log = batchRestApi.getBatchLocalLog(
+            batchId,
+            from,
+            size)
+          from += log.getLogRowSet.size
+          log.getLogRowSet.asScala.foreach(x => info(x))
+
+          batch = batchRestApi.getBatchById(batchId)
+          if (log.getLogRowSet.size() == 0 && batch.getState() != "PENDING"
+            && batch.getState() != "RUNNING") {
+            done = true
+          }
+        }

Review Comment:
   we can do some enhancements in **`another pr for follow up`**:
   
   1. get the kyuubi instance that created this batch
   2. always call the kyuubi instance that created this batch to get local logs to prevent request redirection across kyuubi instances
   3. if the request(get state) to the kyuubi instance that created this batch failed, call the base url(might be a VIP for load balance and HA)



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/Helper.scala:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.util
+
+import java.io.{BufferedReader, File, FileInputStream, InputStreamReader}
+import java.nio.charset.StandardCharsets
+import java.util.HashMap
+
+import org.yaml.snakeyaml.Yaml
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_SHARE_LEVEL, ENGINE_SHARE_LEVEL_SUBDOMAIN, ENGINE_TYPE}
+import org.apache.kyuubi.ctl.{CliConfig, ControlObject}
+import org.apache.kyuubi.ha.client.{DiscoveryClient, DiscoveryPaths, ServiceNodeInfo}
+import org.apache.kyuubi.ha.client.DiscoveryClientProvider.withDiscoveryClient
+
+object Helper {

Review Comment:
   is there a more appropriate name? 



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


[GitHub] [incubator-kyuubi] lightning-L commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
lightning-L commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895097105


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -62,16 +68,36 @@ object CommandLine {
           " change it if the active service is running in another."),
       opt[Unit]('b', "verbose")
         .action((_, c) => c.copy(commonOpts = c.commonOpts.copy(verbose = true)))
-        .text("Print additional debug output."))
+        .text("Print additional debug output."),
+      opt[String]("hostUrl")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(hostUrl = v)))
+        .text("Host url for rest api."),
+      opt[String]("authSchema")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(authSchema = v)))
+        .text("Auth schema for rest api."),
+      opt[String]("username")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(username = v)))
+        .text("Username for basic authentication."),
+      opt[String]("password")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(password = v)))
+        .text("Password for rest api."),
+      opt[String]("spnegoHost")
+        .action((v, c) => c.copy(commonOpts = c.commonOpts.copy(spnegoHost = v)))
+        .text("Spnego host for rest api."))
   }
 
   private def create(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
     import builder._
     OParser.sequence(
       note(""),
       cmd("create")
-        .action((_, c) => c.copy(action = ServiceControlAction.CREATE))
+        .text("\tCreate a resource.")
+        .action((_, c) => c.copy(action = ControlAction.CREATE))
         .children(
+          opt[String]('f', "filename")
+            .action((v, c) => c.copy(createOpts = c.createOpts.copy(filename = v)))
+            .text("Filename to use to create the resource"),

Review Comment:
   Seems the `\t` is added only for `cmd`, not for `opt`.
   Please refer to the test case (test --help) in `ControlCliArgumentsSuite.scala`



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


[GitHub] [incubator-kyuubi] lightning-L commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
lightning-L commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895181071


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -137,4 +194,94 @@ object CommandLine {
           .text("The engine share level this engine belong to."))
   }
 
+  private def batchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+  }
+
+  private def getBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")

Review Comment:
   <img width="1204" alt="image" src="https://user-images.githubusercontent.com/21362040/173237339-dacc912e-2d80-402b-8a28-3ab86a70fe7f.png">
   



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


[GitHub] [incubator-kyuubi] lightning-L commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
lightning-L commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895192570


##########
kyuubi-ctl/src/test/scala/org/apache/kyuubi/ctl/ControlCliArgumentsTestUtil.scala:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.ctl
+
+import org.apache.kyuubi.KyuubiFunSuite
+
+object ControlCliArgumentsTestUtil extends KyuubiFunSuite {
+
+  /** Check whether the script exits and the given search string is printed. */
+  def testPrematureExit(args: Array[String], searchString: String): Unit = {

Review Comment:
   <img width="736" alt="image" src="https://user-images.githubusercontent.com/21362040/173239973-5c46337b-cc84-4661-92c8-60a28d3fdd0f.png">
   seems this method only applies to kyuubi-ctl module



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


[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#issuecomment-1152947155

   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2823](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f20358a) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/ce56d700707434e6c3ee22952ab86121897e1d78?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce56d70) will **increase** coverage by `0.37%`.
   > The diff coverage is `81.34%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2823      +/-   ##
   ============================================
   + Coverage     63.32%   63.70%   +0.37%     
     Complexity      205      205              
   ============================================
     Files           485      501      +16     
     Lines         21692    22098     +406     
     Branches       2859     2919      +60     
   ============================================
   + Hits          13737    14078     +341     
   - Misses         6655     6697      +42     
   - Partials       1300     1323      +23     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...la/org/apache/kyuubi/ctl/ControlCliArguments.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvQ29udHJvbENsaUFyZ3VtZW50cy5zY2FsYQ==) | `32.50% <21.31%> (-22.82%)` | :arrow_down: |
   | [...main/scala/org/apache/kyuubi/ctl/cmd/Command.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL0NvbW1hbmQuc2NhbGE=) | `57.89% <38.88%> (-16.35%)` | :arrow_down: |
   | [.../main/scala/org/apache/kyuubi/ctl/ControlCli.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvQ29udHJvbENsaS5zY2FsYQ==) | `46.42% <50.00%> (ø)` | |
   | [...apache/kyuubi/ctl/cmd/list/ListEngineCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xpc3QvTGlzdEVuZ2luZUNvbW1hbmQuc2NhbGE=) | `50.00% <50.00%> (ø)` | |
   | [...he/kyuubi/ctl/cmd/create/CreateServerCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2NyZWF0ZS9DcmVhdGVTZXJ2ZXJDb21tYW5kLnNjYWxh) | `88.88% <66.66%> (ø)` | |
   | [...n/scala/org/apache/kyuubi/ctl/util/Validator.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvdXRpbC9WYWxpZGF0b3Iuc2NhbGE=) | `68.00% <68.00%> (ø)` | |
   | [...he/kyuubi/ctl/cmd/delete/DeleteEngineCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2RlbGV0ZS9EZWxldGVFbmdpbmVDb21tYW5kLnNjYWxh) | `75.00% <75.00%> (ø)` | |
   | [...g/apache/kyuubi/ctl/cmd/get/GetEngineCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2dldC9HZXRFbmdpbmVDb21tYW5kLnNjYWxh) | `75.00% <75.00%> (ø)` | |
   | [.../apache/kyuubi/ctl/cmd/list/ListBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xpc3QvTGlzdEJhdGNoQ29tbWFuZC5zY2FsYQ==) | `80.00% <80.00%> (ø)` | |
   | [...rg/apache/kyuubi/ctl/cmd/get/GetBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2dldC9HZXRCYXRjaENvbW1hbmQuc2NhbGE=) | `85.71% <85.71%> (ø)` | |
   | ... and [23 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ce56d70...f20358a](https://codecov.io/gh/apache/incubator-kyuubi/pull/2823?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891208402


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -137,4 +191,72 @@ object CommandLine {
           .text("The engine share level this engine belong to."))
   }
 
+  private def batchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(service = ControlObject.BATCH))
+  }
+
+  private def getBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(service = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."))
+  }
+
+  private def deleteBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(service = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."),
+        opt[String]("hs2ProxyUser")
+          .action((v, c) => c.copy(createOpts = c.createOpts.copy(filename = v)))
+          .text("The value of hive.server2.proxy.user config."))
+  }
+
+  private def listBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(service = ControlObject.BATCH))
+      .children(
+        opt[String]("batchType")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchType = v)))
+          .text("Batch type."),

Review Comment:
   mentioned above for more filter conditions



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891235844


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.log
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.api.v1.dto.{Batch, OperationLog}
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class LogBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  override def validateArguments(): Unit = {
+    if (cliArgs.batchOpts.batchId == null) {
+      fail("Must specify batchId for log batch command.")
+    }
+  }
+
+  override def run(): Unit = {
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, null, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    var log: OperationLog = batchRestApi.getBatchLocalLog(
+      cliArgs.batchOpts.batchId,
+      cliArgs.batchOpts.from,
+      cliArgs.batchOpts.size)
+    log.getLogRowSet.asScala.foreach(x => info(x))
+
+    var done = false
+    val batchId = cliArgs.batchOpts.batchId
+    var batch: Batch = null
+    if (cliArgs.logOpts.forward) {
+      while (!done) {
+        log = batchRestApi.getBatchLocalLog(
+          batchId,
+          -1,
+          cliArgs.batchOpts.size)
+        log.getLogRowSet.asScala.foreach(x => info(x))
+
+        batch = batchRestApi.getBatchById(batchId)
+        if (log.getLogRowSet.size() == 0 && batch.getState() != "RUNNING") {

Review Comment:
   batch.getState() != "PENDING" && batch.getState() != "RUNNING"?



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891235844


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.log
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.api.v1.dto.{Batch, OperationLog}
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class LogBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  override def validateArguments(): Unit = {
+    if (cliArgs.batchOpts.batchId == null) {
+      fail("Must specify batchId for log batch command.")
+    }
+  }
+
+  override def run(): Unit = {
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, null, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    var log: OperationLog = batchRestApi.getBatchLocalLog(
+      cliArgs.batchOpts.batchId,
+      cliArgs.batchOpts.from,
+      cliArgs.batchOpts.size)
+    log.getLogRowSet.asScala.foreach(x => info(x))
+
+    var done = false
+    val batchId = cliArgs.batchOpts.batchId
+    var batch: Batch = null
+    if (cliArgs.logOpts.forward) {
+      while (!done) {
+        log = batchRestApi.getBatchLocalLog(
+          batchId,
+          -1,
+          cliArgs.batchOpts.size)
+        log.getLogRowSet.asScala.foreach(x => info(x))
+
+        batch = batchRestApi.getBatchById(batchId)
+        if (log.getLogRowSet.size() == 0 && batch.getState() != "RUNNING") {

Review Comment:
   log.getLogRowSet.size() == 0 && batch.getState() != "PENDING" && batch.getState() != "RUNNING"?



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891230058


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/list/ListBatchCommand.scala:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.list
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.api.v1.dto.GetBatchesResponse
+import org.apache.kyuubi.client.util.JsonUtil
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class ListBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  override def validateArguments(): Unit = {}
+
+  override def run(): Unit = {
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, null, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    val batchInfoList: GetBatchesResponse = batchRestApi.listBatches(
+      cliArgs.batchOpts.batchType,
+      null,
+      null,
+      null,
+      null,
+      if (cliArgs.batchOpts.from < 0) 0 else cliArgs.batchOpts.from,
+      cliArgs.batchOpts.size)
+    info(JsonUtil.toJson(batchInfoList))
+
+    kyuubiRestClient.close()

Review Comment:
   ditto



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891236413


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.log
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.api.v1.dto.{Batch, OperationLog}
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class LogBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  override def validateArguments(): Unit = {
+    if (cliArgs.batchOpts.batchId == null) {
+      fail("Must specify batchId for log batch command.")
+    }
+  }
+
+  override def run(): Unit = {
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, null, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    var log: OperationLog = batchRestApi.getBatchLocalLog(
+      cliArgs.batchOpts.batchId,
+      cliArgs.batchOpts.from,
+      cliArgs.batchOpts.size)
+    log.getLogRowSet.asScala.foreach(x => info(x))
+
+    var done = false
+    val batchId = cliArgs.batchOpts.batchId
+    var batch: Batch = null
+    if (cliArgs.logOpts.forward) {
+      while (!done) {
+        log = batchRestApi.getBatchLocalLog(
+          batchId,
+          -1,
+          cliArgs.batchOpts.size)
+        log.getLogRowSet.asScala.foreach(x => info(x))
+
+        batch = batchRestApi.getBatchById(batchId)
+        if (log.getLogRowSet.size() == 0 && batch.getState() != "RUNNING") {
+          done = true
+        }
+      }
+    }
+
+    kyuubiRestClient.close()

Review Comment:
   withKyuubiRestClient



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891229741


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/get/GetBatchCommand.scala:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.get
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.api.v1.dto.Batch
+import org.apache.kyuubi.client.util.JsonUtil
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class GetBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  override def validateArguments(): Unit = {
+    if (cliArgs.batchOpts.batchId == null) {
+      fail("Must specify batchId for get batch command.")
+    }
+  }
+
+  override def run(): Unit = {
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, null, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    val batch: Batch = batchRestApi.getBatchById(cliArgs.batchOpts.batchId)
+    info(JsonUtil.toJson(batch))
+
+    kyuubiRestClient.close()

Review Comment:
   ditto



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


[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891269803


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ControlCliArguments.scala:
##########
@@ -57,18 +59,28 @@ class ControlCliArguments(args: Seq[String], env: Map[String, String] = sys.env)
   }
 
   private def getCommand(cliArgs: CliConfig): Command = {
-    cliArgs.action match {
-      case ServiceControlAction.CREATE => new CreateCommand(cliArgs)
-      case ServiceControlAction.GET => new GetCommand(cliArgs)
-      case ServiceControlAction.DELETE => new DeleteCommand(cliArgs)
-      case ServiceControlAction.LIST => new ListCommand(cliArgs)
-      case _ => null
-    }
+    val operationName = cliArgs.action.toString.toLowerCase
+    val resourceName = cliArgs.service.toString.toLowerCase
+
+    val packageName = operationName
+    val commandName = s"${operationName.capitalize}${resourceName.capitalize}"
+    val className = s"org.apache.kyuubi.ctl.cmd.${packageName}.${commandName}Command"
+
+    val mirror: ru.Mirror = ru.runtimeMirror(getClass.getClassLoader)
+    val classSymbol: ru.ClassSymbol = mirror.staticClass(className)
+
+    val consMethodSymbol = classSymbol.primaryConstructor.asMethod
+
+    val classMirror = mirror.reflectClass(classSymbol)
+    val consMethodMirror = classMirror.reflectConstructor(consMethodSymbol)
+
+    val result = consMethodMirror.apply(cliArgs).asInstanceOf[Command]
+    result

Review Comment:
   It seems `match case` is more readable



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891253143


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.log
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.api.v1.dto.{Batch, OperationLog}
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class LogBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  override def validateArguments(): Unit = {
+    if (cliArgs.batchOpts.batchId == null) {
+      fail("Must specify batchId for log batch command.")
+    }
+  }
+
+  override def run(): Unit = {
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, null, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    var log: OperationLog = batchRestApi.getBatchLocalLog(
+      cliArgs.batchOpts.batchId,
+      cliArgs.batchOpts.from,
+      cliArgs.batchOpts.size)
+    log.getLogRowSet.asScala.foreach(x => info(x))
+
+    var done = false
+    val batchId = cliArgs.batchOpts.batchId
+    var batch: Batch = null
+    if (cliArgs.logOpts.forward) {
+      while (!done) {
+        log = batchRestApi.getBatchLocalLog(
+          batchId,
+          -1,

Review Comment:
   I see, if the from < 0, the server will use the `lastSeekReadPos`.
   
   But to prevent concurrent conflicts, I think we can calculate the new offset based on last offset + total size.
   



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895147406


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -137,4 +194,94 @@ object CommandLine {
           .text("The engine share level this engine belong to."))
   }
 
+  private def batchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+  }
+
+  private def getBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."))
+  }
+
+  private def deleteBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."),
+        opt[String]("hs2ProxyUser")
+          .action((v, c) => c.copy(createOpts = c.createOpts.copy(filename = v)))
+          .text("The value of hive.server2.proxy.user config."))
+  }
+
+  private def listBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        opt[String]("batchType")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchType = v)))
+          .text("Batch type."),
+        opt[String]("batchUser")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchUser = v)))
+          .text("Batch user."),
+        opt[String]("batchState")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchState = v)))
+          .text("Batch state."),
+        opt[String]("createTime")
+          .action((v, c) =>
+            c.copy(batchOpts = c.batchOpts.copy(createTime =
+              dateStringToMillis(v, "yyyyMMddHHmmss"))))
+          .validate(x =>
+            if (x.matches("\\d{14}")) success
+            else failure("Option --createTime must be in yyyyMMddHHmmss format."))
+          .text("Batch create time, should be in yyyyMMddHHmmss format."),
+        opt[String]("endTime")
+          .action((v, c) =>
+            c.copy(batchOpts = c.batchOpts.copy(endTime =
+              dateStringToMillis(v, "yyyyMMddHHmmss"))))
+          .validate(x =>
+            if (x.matches("\\d{14}")) success
+            else failure("Option --endTime must be in yyyyMMddHHmmss format."))

Review Comment:
   ```
   if () {
   } else {
   }
   ```



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -137,4 +194,94 @@ object CommandLine {
           .text("The engine share level this engine belong to."))
   }
 
+  private def batchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+  }
+
+  private def getBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."))
+  }
+
+  private def deleteBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."),
+        opt[String]("hs2ProxyUser")
+          .action((v, c) => c.copy(createOpts = c.createOpts.copy(filename = v)))
+          .text("The value of hive.server2.proxy.user config."))
+  }
+
+  private def listBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        opt[String]("batchType")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchType = v)))
+          .text("Batch type."),
+        opt[String]("batchUser")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchUser = v)))
+          .text("Batch user."),
+        opt[String]("batchState")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchState = v)))
+          .text("Batch state."),
+        opt[String]("createTime")
+          .action((v, c) =>
+            c.copy(batchOpts = c.batchOpts.copy(createTime =
+              dateStringToMillis(v, "yyyyMMddHHmmss"))))
+          .validate(x =>
+            if (x.matches("\\d{14}")) success
+            else failure("Option --createTime must be in yyyyMMddHHmmss format."))
+          .text("Batch create time, should be in yyyyMMddHHmmss format."),
+        opt[String]("endTime")
+          .action((v, c) =>
+            c.copy(batchOpts = c.batchOpts.copy(endTime =
+              dateStringToMillis(v, "yyyyMMddHHmmss"))))
+          .validate(x =>
+            if (x.matches("\\d{14}")) success
+            else failure("Option --endTime must be in yyyyMMddHHmmss format."))
+          .text("Batch end time, should be in yyyyMMddHHmmss format."),
+        opt[Int]("from")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(from = v)))
+          .validate(x =>
+            if (x >= 0) success
+            else failure("Option --from must be >=0"))

Review Comment:
   ditto



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895149622


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/RestClientFactory.scala:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl
+
+import java.util.{Map => JMap}
+
+import org.apache.commons.lang3.StringUtils
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.client.KyuubiRestClient
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.ctl.CtlConf._
+
+object RestClientFactory {
+
+  private[ctl] def withKyuubiRestClient(
+      cliConfig: CliConfig,
+      map: JMap[String, Object],
+      conf: KyuubiConf)(f: KyuubiRestClient => Unit): Unit = {
+    val kyuubiRestClient: KyuubiRestClient =
+      RestClientFactory.getKyuubiRestClient(cliConfig, map, conf)
+    try {
+      f(kyuubiRestClient)
+    } finally {
+      kyuubiRestClient.close()
+    }
+  }
+
+  private def getKyuubiRestClient(
+      cliConfig: CliConfig,
+      map: JMap[String, Object],
+      conf: KyuubiConf): KyuubiRestClient = {
+    val version = getApiVersion(map)
+    val hostUrl = getRestConfig("hostUrl", conf.get(CTL_REST_CLIENT_BASE_URL).get, cliConfig, map)
+    val authSchema =
+      getRestConfig("authSchema", conf.get(CTL_REST_CLIENT_AUTH_SCHEMA), cliConfig, map)
+
+    var kyuubiRestClient: KyuubiRestClient = null
+    authSchema match {
+      case "basic" =>

Review Comment:
   it should be case insensitive



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891170975


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -600,6 +600,28 @@ object KyuubiConf {
       .version("1.4.0")
       .fallbackConf(FRONTEND_WORKER_KEEPALIVE_TIME)
 
+  val REST_CLIENT_BASE_URL: OptionalConfigEntry[String] =
+    buildConf("kyuubi.rest.base.url")

Review Comment:
   Move these config items to `CtlConf`, please refer `HighAvailabilityConf`.



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891169931


##########
kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala:
##########
@@ -600,6 +600,28 @@ object KyuubiConf {
       .version("1.4.0")
       .fallbackConf(FRONTEND_WORKER_KEEPALIVE_TIME)
 
+  val REST_CLIENT_BASE_URL: OptionalConfigEntry[String] =
+    buildConf("kyuubi.rest.base.url")

Review Comment:
   kyuubi.rest.** => kyuubi.ctl.rest.**



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891199423


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ClientFactory.scala:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl
+
+import java.util.HashMap
+
+import scala.reflect.runtime.{universe => ru}
+import scala.reflect.runtime.universe._
+
+import org.apache.commons.lang3.StringUtils
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.client.KyuubiRestClient
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+
+object ClientFactory {
+
+  private[ctl] def getKyuubiRestClient(
+      cliArgs: CliConfig,
+      map: HashMap[String, Object],
+      conf: KyuubiConf): KyuubiRestClient = {
+    val version = getApiVersion(map)
+    val hostUrl = getRestConfig("hostUrl", conf.get(REST_CLIENT_BASE_URL).get, cliArgs, map)
+    val authSchema = getRestConfig("authSchema", conf.get(REST_CLIENT_AUTH_SCHEMA), cliArgs, map)
+
+    var kyuubiRestClient: KyuubiRestClient = null
+    authSchema match {
+      case "basic" =>
+        val username = getRestConfig("username", null, cliArgs, map)
+        val password = cliArgs.commonOpts.password
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.BASIC)
+          .username(username)
+          .password(password)
+          .build()
+      case "spnego" =>
+        val spnegoHost =
+          getRestConfig("spnegoHost", conf.get(REST_CLIENT_SPNEGO_HOST).get, cliArgs, map)
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.SPNEGO)
+          .spnegoHost(spnegoHost)
+          .build()
+      case _ => throw new KyuubiException(s"Unsupported auth schema: $authSchema")
+    }
+    kyuubiRestClient
+  }
+
+  private def getApiVersion(map: HashMap[String, Object]): String = {
+    var version: String = "V1"
+    if (map != null) {
+      val configuredVersion = map.get("apiVersion").asInstanceOf[String].toUpperCase
+      if (StringUtils.isNotBlank(configuredVersion)) {
+        version = configuredVersion
+      }
+    }
+    version
+  }
+
+  private def getRestConfig(
+      key: String,
+      defaultValue: String,
+      cliArgs: CliConfig,
+      map: HashMap[String, Object]): String = {
+    var configValue: String = null
+    // get value from command line
+    val commonOpts = cliArgs.commonOpts
+    val mirror = ru.runtimeMirror(getClass.getClassLoader)
+    val instanceMirror = mirror.reflect(commonOpts)
+    val field = typeOf[CommonOpts].decl(TermName(key)).asTerm.accessed.asTerm
+    val fieldMirror = instanceMirror.reflectField(field)
+    configValue = fieldMirror.get.asInstanceOf[String]

Review Comment:
   you can use the `product*` methods of scala case class. 



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895149775


##########
kyuubi-ctl/src/test/scala/org/apache/kyuubi/ctl/ControlCliArgumentsSuite.scala:
##########
@@ -362,14 +347,25 @@ class ControlCliArgumentsSuite extends KyuubiFunSuite {
          |  -p, --port <value>       Listening port of a service.
          |  -v, --version <value>    $versionHelpString
          |  -b, --verbose            Print additional debug output.
+         |  --hostUrl <value>        Host url for rest api.
+         |  --authSchema <value>     Auth schema for rest api.

Review Comment:
   Auth schema for rest api, valid values are basic, spnego.



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


[GitHub] [incubator-kyuubi] lightning-L commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
lightning-L commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895195466


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ControlCliArguments.scala:
##########
@@ -48,48 +54,88 @@ class ControlCliArguments(args: Seq[String], env: Map[String, String] = sys.env)
         result match {
           case Some(arguments) =>
             command = getCommand(arguments)
-            command.preProcess()
-            cliArgs = command.cliArgs
+            command.validate()

Review Comment:
   when command in not valid, the parser will execute the runEffects logic.
   ctl will termindate and throw prompt like "Unknown argument xxx".
   <img width="667" alt="image" src="https://user-images.githubusercontent.com/21362040/173240983-4bfc5079-e29b-43a0-b92b-e1dc25e4aa44.png">
   
   when getCommand is executed, the command should be valid.



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895267715


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/config/AllKyuubiConfiguration.scala:
##########
@@ -70,6 +71,7 @@ class AllKyuubiConfiguration extends KyuubiFunSuite {
   test("Check all kyuubi configs") {
     KyuubiConf
     JDBCStateStoreConf
+    CtlConf

Review Comment:
   now the UT for `AllKyuubiConfiguration` failed, please re-generate it.



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895185168


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -137,4 +194,94 @@ object CommandLine {
           .text("The engine share level this engine belong to."))
   }
 
+  private def batchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+  }
+
+  private def getBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")

Review Comment:
   got it. thanks



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891229268


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/delete/DeleteBatchCommand.scala:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.delete
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.util.JsonUtil
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class DeleteBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  var result: String = null
+
+  def validateArguments(): Unit = {}
+
+  override def run(): Unit = {
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, null, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    val result = batchRestApi.deleteBatch(
+      cliArgs.batchOpts.batchId,
+      cliArgs.batchOpts.hs2ProxyUser)
+    info(JsonUtil.toJson(result))
+  }

Review Comment:
   ditto



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891218383


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/Command.scala:
##########
@@ -155,6 +162,33 @@ abstract class Command(var cliArgs: CliConfig) extends Logging {
     }
   }
 
+  private[ctl] def readConfig(): HashMap[String, Object] = {
+    var filename = cliArgs.createOpts.filename
+    if (StringUtils.isBlank(filename)) {
+      fail(s"Config file is not specified.")
+    }
+
+    val currentPath = Paths.get("").toAbsolutePath
+    if (!filename.startsWith("/")) {
+      filename = currentPath + "/" + filename
+    }
+    if (!Files.exists(Paths.get(filename))) {
+      fail(s"Config file does not exist: ${filename}.")
+    }
+
+    var map: HashMap[String, Object] = null
+
+    try {
+      val yaml = new Yaml()
+      val input = new FileInputStream(new File(filename))
+      val br = new BufferedReader(new InputStreamReader(input, StandardCharsets.UTF_8))
+      map = yaml.load(br).asInstanceOf[HashMap[String, Object]]
+    } catch {
+      case e: Exception => fail(s"Failed to read yaml file[$filename]: $e")
+    }

Review Comment:
   finally {
   br.close()
   }



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891215126


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ControlCliArguments.scala:
##########
@@ -79,7 +91,7 @@ class ControlCliArguments(args: Seq[String], env: Map[String, String] = sys.env)
            |  version                 ${cliArgs.commonOpts.version}
            |  verbose                 ${cliArgs.commonOpts.verbose}
         """.stripMargin
-      case ServiceControlObject.ENGINE =>
+      case ControlObject.ENGINE =>

Review Comment:
   need add `ControlObject.BATCH`



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895154181


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ControlCliArguments.scala:
##########
@@ -48,48 +54,88 @@ class ControlCliArguments(args: Seq[String], env: Map[String, String] = sys.env)
         result match {
           case Some(arguments) =>
             command = getCommand(arguments)
-            command.preProcess()
-            cliArgs = command.cliArgs
+            command.validate()

Review Comment:
   here command might be null, it is better to provide some prompt instead of NPE



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895147441


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -137,4 +194,94 @@ object CommandLine {
           .text("The engine share level this engine belong to."))
   }
 
+  private def batchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+  }
+
+  private def getBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."))
+  }
+
+  private def deleteBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."),
+        opt[String]("hs2ProxyUser")
+          .action((v, c) => c.copy(createOpts = c.createOpts.copy(filename = v)))
+          .text("The value of hive.server2.proxy.user config."))
+  }
+
+  private def listBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        opt[String]("batchType")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchType = v)))
+          .text("Batch type."),
+        opt[String]("batchUser")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchUser = v)))
+          .text("Batch user."),
+        opt[String]("batchState")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchState = v)))
+          .text("Batch state."),
+        opt[String]("createTime")
+          .action((v, c) =>
+            c.copy(batchOpts = c.batchOpts.copy(createTime =
+              dateStringToMillis(v, "yyyyMMddHHmmss"))))
+          .validate(x =>
+            if (x.matches("\\d{14}")) success
+            else failure("Option --createTime must be in yyyyMMddHHmmss format."))
+          .text("Batch create time, should be in yyyyMMddHHmmss format."),
+        opt[String]("endTime")
+          .action((v, c) =>
+            c.copy(batchOpts = c.batchOpts.copy(endTime =
+              dateStringToMillis(v, "yyyyMMddHHmmss"))))
+          .validate(x =>
+            if (x.matches("\\d{14}")) success
+            else failure("Option --endTime must be in yyyyMMddHHmmss format."))
+          .text("Batch end time, should be in yyyyMMddHHmmss format."),
+        opt[Int]("from")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(from = v)))
+          .validate(x =>
+            if (x >= 0) success
+            else failure("Option --from must be >=0"))
+          .text("Specify which record to start from retrieving info."),
+        opt[Int]("size")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(size = v)))
+          .validate(x =>
+            if (x >= 0) success
+            else failure("Option --size must be >=0"))

Review Comment:
   ditto



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895149834


##########
kyuubi-ctl/src/test/scala/org/apache/kyuubi/ctl/ControlCliArgumentsSuite.scala:
##########
@@ -362,14 +347,25 @@ class ControlCliArgumentsSuite extends KyuubiFunSuite {
          |  -p, --port <value>       Listening port of a service.
          |  -v, --version <value>    $versionHelpString
          |  -b, --verbose            Print additional debug output.
+         |  --hostUrl <value>        Host url for rest api.
+         |  --authSchema <value>     Auth schema for rest api.
+         |  --username <value>       Username for basic authentication.
+         |  --password <value>       Password for rest api.

Review Comment:
   Password for basic authentication.



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


[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895335627


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/submit/SubmitBatchCommand.scala:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.submit
+
+import java.util.{ArrayList, HashMap}
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.BatchRestApi
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest, OperationLog}
+import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
+import org.apache.kyuubi.ctl.cmd.Command
+import org.apache.kyuubi.ctl.util.{CtlUtils, Validator}
+
+class SubmitBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  def validate(): Unit = {
+    Validator.validateFilename(normalizedCliConfig)
+  }
+
+  def run(): Unit = {
+    val map = CtlUtils.loadYamlAsMap(normalizedCliConfig)
+
+    withKyuubiRestClient(normalizedCliConfig, map, conf) { kyuubiRestClient =>
+      val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+      val request = map.get("request").asInstanceOf[HashMap[String, Object]]
+      val batchRequest = new BatchRequest(
+        map.get("batchType").asInstanceOf[String],
+        request.get("resource").asInstanceOf[String],
+        request.get("className").asInstanceOf[String],
+        request.get("name").asInstanceOf[String],
+        request.get("configs").asInstanceOf[HashMap[String, String]],
+        request.get("args").asInstanceOf[ArrayList[String]])
+
+      var batch: Batch = batchRestApi.createBatch(batchRequest)
+      val batchId = batch.getId
+      var log: OperationLog = null
+      var done = false
+      var from = 0
+      val size = 20
+      while (!done) {

Review Comment:
   I'm ok to improve in another pr



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895029920


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/submit/SubmitBatchCommand.scala:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.submit
+
+import java.util.{ArrayList, HashMap}
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.BatchRestApi
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest, OperationLog}
+import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
+import org.apache.kyuubi.ctl.cmd.Command
+import org.apache.kyuubi.ctl.util.{Helper, Validator}
+
+class SubmitBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  def validate(): Unit = {
+    Validator.validateFilename(normalizedCliConfig)
+  }
+
+  def run(): Unit = {
+    val map: HashMap[String, Object] = Helper.loadYamlIntoMap(normalizedCliConfig)
+
+    withKyuubiRestClient(normalizedCliConfig, map, conf) { kyuubiRestClient =>
+      val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+      val request = map.get("request").asInstanceOf[HashMap[String, Object]]
+      val batchRequest = new BatchRequest(
+        map.get("batchType").asInstanceOf[String],
+        request.get("resource").asInstanceOf[String],
+        request.get("className").asInstanceOf[String],
+        request.get("name").asInstanceOf[String],
+        request.get("configs").asInstanceOf[HashMap[String, String]],
+        request.get("args").asInstanceOf[ArrayList[String]])
+
+      var batch: Batch = batchRestApi.createBatch(batchRequest)
+      val batchId = batch.getId
+      var log: OperationLog = null
+      var done = false
+      var from = 0
+      val size = 20
+      while (!done) {
+        log = batchRestApi.getBatchLocalLog(
+          batchId,
+          from,
+          size)
+        from += log.getLogRowSet.size
+        log.getLogRowSet.asScala.foreach(x => info(x))
+
+        batch = batchRestApi.getBatchById(batchId)
+        if (log.getLogRowSet.size() == 0 && batch.getState() != "PENDING"
+          && batch.getState() != "RUNNING") {
+          done = true
+        }
+      }

Review Comment:
   we can do some enhancements in **`another pr for follow up`**:
   
   1. get the kyuubi instance that created this batch
   2. always call the kyuubi instance that created this batch to get local logs to prevent request redirection across kyuubi instances
   3. if the request(get state) to the kyuubi instance that created this batch failed, call the base url(might be a VIP for load balance and HA)



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891253143


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala:
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.log
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.api.v1.dto.{Batch, OperationLog}
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class LogBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  override def validateArguments(): Unit = {
+    if (cliArgs.batchOpts.batchId == null) {
+      fail("Must specify batchId for log batch command.")
+    }
+  }
+
+  override def run(): Unit = {
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, null, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    var log: OperationLog = batchRestApi.getBatchLocalLog(
+      cliArgs.batchOpts.batchId,
+      cliArgs.batchOpts.from,
+      cliArgs.batchOpts.size)
+    log.getLogRowSet.asScala.foreach(x => info(x))
+
+    var done = false
+    val batchId = cliArgs.batchOpts.batchId
+    var batch: Batch = null
+    if (cliArgs.logOpts.forward) {
+      while (!done) {
+        log = batchRestApi.getBatchLocalLog(
+          batchId,
+          -1,

Review Comment:
   I see, if the from < 0, the server will use the `lastSeekReadPos`.
   
   But to prevent that concurrent conflicts, I think we can calculate the new offset based on last offset + total size.
   



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891206889


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -106,22 +131,51 @@ object CommandLine {
     OParser.sequence(
       note(""),
       cmd("list")
-        .text("\tList all the service/engine nodes for a particular domain.")
-        .action((_, c) => c.copy(action = ServiceControlAction.LIST))
+        .text("\tList information about resources.")
+        .action((_, c) => c.copy(action = ControlAction.LIST))
         .children(
+          listBatchCmd(builder).text("\tList batch session info."),
           serverCmd(builder).text("\tList all the service nodes for a particular domain"),
           engineCmd(builder).text("\tList all the engine nodes for a user")))
 
   }
 
+  private def log(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    OParser.sequence(
+      note(""),
+      cmd("log")
+        .text("\tPrint the logs for specified resource.")
+        .action((_, c) => c.copy(action = ControlAction.LOG))
+        .children(
+          opt[Unit]("forward")
+            .action((_, c) => c.copy(logOpts = c.logOpts.copy(forward = true)))
+            .text("If forward is specified, the ctl will block forever."),
+          logBatchCmd(builder).text("\tGet batch session local log.")))
+  }
+
+  private def submit(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    OParser.sequence(
+      note(""),
+      cmd("submit")
+        .text("\tCombination of create and log command.")

Review Comment:
   Combination of create, get and  log commands.



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895328522


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/submit/SubmitBatchCommand.scala:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.submit
+
+import java.util.{ArrayList, HashMap}
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.BatchRestApi
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest, OperationLog}
+import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
+import org.apache.kyuubi.ctl.cmd.Command
+import org.apache.kyuubi.ctl.util.{CtlUtils, Validator}
+
+class SubmitBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  def validate(): Unit = {
+    Validator.validateFilename(normalizedCliConfig)
+  }
+
+  def run(): Unit = {
+    val map = CtlUtils.loadYamlAsMap(normalizedCliConfig)
+
+    withKyuubiRestClient(normalizedCliConfig, map, conf) { kyuubiRestClient =>
+      val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+      val request = map.get("request").asInstanceOf[HashMap[String, Object]]
+      val batchRequest = new BatchRequest(
+        map.get("batchType").asInstanceOf[String],
+        request.get("resource").asInstanceOf[String],
+        request.get("className").asInstanceOf[String],
+        request.get("name").asInstanceOf[String],
+        request.get("configs").asInstanceOf[HashMap[String, String]],
+        request.get("args").asInstanceOf[ArrayList[String]])
+
+      var batch: Batch = batchRestApi.createBatch(batchRequest)
+      val batchId = batch.getId
+      var log: OperationLog = null
+      var done = false
+      var from = 0
+      val size = 20
+      while (!done) {

Review Comment:
   yes, we need it.



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895148626


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CtlConf.scala:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.ctl
+
+import org.apache.kyuubi.config.{ConfigBuilder, ConfigEntry, KyuubiConf, OptionalConfigEntry}
+
+object CtlConf {
+
+  private def buildConf(key: String): ConfigBuilder = KyuubiConf.buildConf(key)
+
+  val CTL_REST_CLIENT_BASE_URL: OptionalConfigEntry[String] =
+    buildConf("kyuubi.ctl.rest.base.url")
+      .doc("The REST API base URL," +
+        "which contains the scheme (http:// or https://), host name, port number")

Review Comment:
   nit: `" which`



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895150150


##########
kyuubi-ctl/src/test/scala/org/apache/kyuubi/ctl/ControlCliArgumentsTestUtil.scala:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.ctl
+
+import org.apache.kyuubi.KyuubiFunSuite
+
+object ControlCliArgumentsTestUtil extends KyuubiFunSuite {
+
+  /** Check whether the script exits and the given search string is printed. */
+  def testPrematureExit(args: Array[String], searchString: String): Unit = {

Review Comment:
   how about removing `ControlCliArgumentsTestUtil` and add `testPrematureExit` into `KyuubiFunSuite`?



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895265131


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ControlCliArguments.scala:
##########
@@ -48,48 +54,88 @@ class ControlCliArguments(args: Seq[String], env: Map[String, String] = sys.env)
         result match {
           case Some(arguments) =>
             command = getCommand(arguments)
-            command.preProcess()
-            cliArgs = command.cliArgs
+            command.validate()

Review Comment:
   got it



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895266509


##########
kyuubi-ctl/src/test/scala/org/apache/kyuubi/ctl/ControlCliArgumentsTestUtil.scala:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.ctl
+
+import org.apache.kyuubi.KyuubiFunSuite
+
+object ControlCliArgumentsTestUtil extends KyuubiFunSuite {
+
+  /** Check whether the script exits and the given search string is printed. */
+  def testPrematureExit(args: Array[String], searchString: String): Unit = {

Review Comment:
   create a class named `KyuubiCtlFunSuite` and  extend it?



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


[GitHub] [incubator-kyuubi] turboFei commented on pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#issuecomment-1154664490

   thanks, merging to master


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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891214268


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ControlCliArguments.scala:
##########
@@ -57,18 +59,28 @@ class ControlCliArguments(args: Seq[String], env: Map[String, String] = sys.env)
   }
 
   private def getCommand(cliArgs: CliConfig): Command = {
-    cliArgs.action match {
-      case ServiceControlAction.CREATE => new CreateCommand(cliArgs)
-      case ServiceControlAction.GET => new GetCommand(cliArgs)
-      case ServiceControlAction.DELETE => new DeleteCommand(cliArgs)
-      case ServiceControlAction.LIST => new ListCommand(cliArgs)
-      case _ => null
-    }
+    val operationName = cliArgs.action.toString.toLowerCase
+    val resourceName = cliArgs.service.toString.toLowerCase
+
+    val packageName = operationName
+    val commandName = s"${operationName.capitalize}${resourceName.capitalize}"
+    val className = s"org.apache.kyuubi.ctl.cmd.${packageName}.${commandName}Command"
+
+    val mirror: ru.Mirror = ru.runtimeMirror(getClass.getClassLoader)
+    val classSymbol: ru.ClassSymbol = mirror.staticClass(className)
+
+    val consMethodSymbol = classSymbol.primaryConstructor.asMethod
+
+    val classMirror = mirror.reflectClass(classSymbol)
+    val consMethodMirror = classMirror.reflectConstructor(consMethodSymbol)
+
+    val result = consMethodMirror.apply(cliArgs).asInstanceOf[Command]
+    result

Review Comment:
   should we continue to use `match`? cc @ulysses-you 



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891216159


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CliConfig.scala:
##########
@@ -16,23 +16,26 @@
  */
 package org.apache.kyuubi.ctl
 
-import org.apache.kyuubi.ctl.ServiceControlAction.ControlAction
-import org.apache.kyuubi.ctl.ServiceControlObject.ControlObject
+import org.apache.kyuubi.ctl.ControlAction.ControlAction
+import org.apache.kyuubi.ctl.ControlObject.ControlObject
 
-private[ctl] object ServiceControlAction extends Enumeration {
+private[ctl] object ControlAction extends Enumeration {
   type ControlAction = Value
-  val CREATE, GET, DELETE, LIST = Value
+  val CREATE, GET, DELETE, LIST, LOG, SUBMIT = Value
 }
 
-private[ctl] object ServiceControlObject extends Enumeration {
+private[ctl] object ControlObject extends Enumeration {
   type ControlObject = Value
-  val SERVER, ENGINE = Value
+  val SERVER, ENGINE, BATCH = Value
 }
 
 case class CliConfig(
     action: ControlAction = null,
-    service: ControlObject = ServiceControlObject.SERVER,
+    service: ControlObject = ControlObject.SERVER,

Review Comment:
   service => resource?



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


[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895316652


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ControlCliArguments.scala:
##########
@@ -48,48 +54,88 @@ class ControlCliArguments(args: Seq[String], env: Map[String, String] = sys.env)
         result match {
           case Some(arguments) =>
             command = getCommand(arguments)
-            command.preProcess()
-            cliArgs = command.cliArgs
+            command.validate()
+            cliConfig = command.normalizedCliConfig
           case _ =>
           // arguments are bad, exit
         }
     }
   }
 
-  private def getCommand(cliArgs: CliConfig): Command = {
-    cliArgs.action match {
-      case ServiceControlAction.CREATE => new CreateCommand(cliArgs)
-      case ServiceControlAction.GET => new GetCommand(cliArgs)
-      case ServiceControlAction.DELETE => new DeleteCommand(cliArgs)
-      case ServiceControlAction.LIST => new ListCommand(cliArgs)
+  private def getCommand(cliConfig: CliConfig): Command = {
+    cliConfig.action match {
+      case ControlAction.CREATE => cliConfig.resource match {
+          case ControlObject.BATCH => new CreateBatchCommand(cliConfig)
+          case ControlObject.SERVER => new CreateServerCommand(cliConfig)
+          case _ => null

Review Comment:
   better to throw exception



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/submit/SubmitBatchCommand.scala:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.submit
+
+import java.util.{ArrayList, HashMap}
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.BatchRestApi
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest, OperationLog}
+import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
+import org.apache.kyuubi.ctl.cmd.Command
+import org.apache.kyuubi.ctl.util.{CtlUtils, Validator}
+
+class SubmitBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  def validate(): Unit = {
+    Validator.validateFilename(normalizedCliConfig)
+  }
+
+  def run(): Unit = {
+    val map = CtlUtils.loadYamlAsMap(normalizedCliConfig)
+
+    withKyuubiRestClient(normalizedCliConfig, map, conf) { kyuubiRestClient =>
+      val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+      val request = map.get("request").asInstanceOf[HashMap[String, Object]]
+      val batchRequest = new BatchRequest(
+        map.get("batchType").asInstanceOf[String],
+        request.get("resource").asInstanceOf[String],
+        request.get("className").asInstanceOf[String],
+        request.get("name").asInstanceOf[String],
+        request.get("configs").asInstanceOf[HashMap[String, String]],
+        request.get("args").asInstanceOf[ArrayList[String]])
+
+      var batch: Batch = batchRestApi.createBatch(batchRequest)
+      val batchId = batch.getId
+      var log: OperationLog = null
+      var done = false
+      var from = 0
+      val size = 20
+      while (!done) {

Review Comment:
   I guess there should have a flag like `waitUntilComplete` ?



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.log
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.BatchRestApi
+import org.apache.kyuubi.client.api.v1.dto.{Batch, OperationLog}
+import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
+import org.apache.kyuubi.ctl.cmd.Command
+
+class LogBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  def validate(): Unit = {
+    if (normalizedCliConfig.batchOpts.batchId == null) {
+      fail("Must specify batchId for log batch command.")
+    }
+  }
+
+  def run(): Unit = {
+    withKyuubiRestClient(normalizedCliConfig, null, conf) { kyuubiRestClient =>
+      val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+      val batchId = normalizedCliConfig.batchOpts.batchId
+      var from = normalizedCliConfig.batchOpts.from
+      val size = normalizedCliConfig.batchOpts.size
+      var log: OperationLog = batchRestApi.getBatchLocalLog(
+        batchId,
+        from,
+        size)
+      log.getLogRowSet.asScala.foreach(x => info(x))
+
+      var done = false
+      var batch: Batch = null
+      from = if (from < 0) log.getLogRowSet.size else from + log.getLogRowSet.size
+      if (normalizedCliConfig.logOpts.forward) {
+        while (!done) {

Review Comment:
   sleep a moment before next fetch



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895267515


##########
kyuubi-server/src/test/scala/org/apache/kyuubi/config/AllKyuubiConfiguration.scala:
##########
@@ -70,6 +71,7 @@ class AllKyuubiConfiguration extends KyuubiFunSuite {
   test("Check all kyuubi configs") {
     KyuubiConf
     JDBCStateStoreConf
+    CtlConf

Review Comment:
   KyuubiConf
   CtlConf
   JDBCStateStoreConf



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895147121


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -137,4 +194,94 @@ object CommandLine {
           .text("The engine share level this engine belong to."))
   }
 
+  private def batchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+  }
+
+  private def getBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")

Review Comment:
   `<batchId>` or `batchId`?



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -137,4 +194,94 @@ object CommandLine {
           .text("The engine share level this engine belong to."))
   }
 
+  private def batchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+  }
+
+  private def getBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."))
+  }
+
+  private def deleteBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")

Review Comment:
   ditto



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


[GitHub] [incubator-kyuubi] turboFei commented on pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#issuecomment-1154665843

   please followup below comments in another prs.
   
   1. support `waitUntilComplete` for submit batch(wait the batch state be running)
   2. for log batch/submit batch, call the kyuubi instance that created batch first, failover to original url if failed
   


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


[GitHub] [incubator-kyuubi] pan3793 commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895349715


##########
kyuubi-ctl/pom.xml:
##########
@@ -32,6 +32,11 @@
     <packaging>jar</packaging>
     <url>https://kyuubi.apache.org/</url>
 
+    <properties>
+        <!-- only used in kyuubi-ctl module -->
+        <snakeyaml.version>1.30</snakeyaml.version>

Review Comment:
   Let manage it as other deps, if you check the `dependencyList`, it's already be there, as a transitive dep of `jackson-dataformat-yaml`



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895031546


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/util/Helper.scala:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.util
+
+import java.io.{BufferedReader, File, FileInputStream, InputStreamReader}
+import java.nio.charset.StandardCharsets
+import java.util.HashMap
+
+import org.yaml.snakeyaml.Yaml
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf.{ENGINE_SHARE_LEVEL, ENGINE_SHARE_LEVEL_SUBDOMAIN, ENGINE_TYPE}
+import org.apache.kyuubi.ctl.{CliConfig, ControlObject}
+import org.apache.kyuubi.ha.client.{DiscoveryClient, DiscoveryPaths, ServiceNodeInfo}
+import org.apache.kyuubi.ha.client.DiscoveryClientProvider.withDiscoveryClient
+
+object Helper {

Review Comment:
   CtlUtils?



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891197693


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/ClientFactory.scala:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl
+
+import java.util.HashMap
+
+import scala.reflect.runtime.{universe => ru}
+import scala.reflect.runtime.universe._
+
+import org.apache.commons.lang3.StringUtils
+
+import org.apache.kyuubi.KyuubiException
+import org.apache.kyuubi.client.KyuubiRestClient
+import org.apache.kyuubi.config.KyuubiConf
+import org.apache.kyuubi.config.KyuubiConf._
+
+object ClientFactory {
+
+  private[ctl] def getKyuubiRestClient(
+      cliArgs: CliConfig,
+      map: HashMap[String, Object],
+      conf: KyuubiConf): KyuubiRestClient = {
+    val version = getApiVersion(map)
+    val hostUrl = getRestConfig("hostUrl", conf.get(REST_CLIENT_BASE_URL).get, cliArgs, map)
+    val authSchema = getRestConfig("authSchema", conf.get(REST_CLIENT_AUTH_SCHEMA), cliArgs, map)
+
+    var kyuubiRestClient: KyuubiRestClient = null
+    authSchema match {
+      case "basic" =>
+        val username = getRestConfig("username", null, cliArgs, map)
+        val password = cliArgs.commonOpts.password
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.BASIC)
+          .username(username)
+          .password(password)
+          .build()
+      case "spnego" =>
+        val spnegoHost =
+          getRestConfig("spnegoHost", conf.get(REST_CLIENT_SPNEGO_HOST).get, cliArgs, map)
+        kyuubiRestClient = KyuubiRestClient.builder(hostUrl)
+          .apiVersion(KyuubiRestClient.ApiVersion.valueOf(version))
+          .authSchema(KyuubiRestClient.AuthSchema.SPNEGO)
+          .spnegoHost(spnegoHost)
+          .build()
+      case _ => throw new KyuubiException(s"Unsupported auth schema: $authSchema")
+    }
+    kyuubiRestClient
+  }
+
+  private def getApiVersion(map: HashMap[String, Object]): String = {
+    var version: String = "V1"
+    if (map != null) {
+      val configuredVersion = map.get("apiVersion").asInstanceOf[String].toUpperCase
+      if (StringUtils.isNotBlank(configuredVersion)) {
+        version = configuredVersion
+      }
+    }
+    version
+  }
+
+  private def getRestConfig(
+      key: String,
+      defaultValue: String,
+      cliArgs: CliConfig,
+      map: HashMap[String, Object]): String = {
+    var configValue: String = null
+    // get value from command line
+    val commonOpts = cliArgs.commonOpts
+    val mirror = ru.runtimeMirror(getClass.getClassLoader)
+    val instanceMirror = mirror.reflect(commonOpts)
+    val field = typeOf[CommonOpts].decl(TermName(key)).asTerm.accessed.asTerm
+    val fieldMirror = instanceMirror.reflectField(field)
+    configValue = fieldMirror.get.asInstanceOf[String]

Review Comment:
   I see. FYI:
   
   ```
   scala> case class C1(v1 :String, v2 :String)
   class C1
   
   scala> val c1Instance = C1("this is v1", "this is v2")
   val c1Instance: C1 = C1(this is v1,this is v2)
   
   scala> val keyNames =  c1Instance.productElementNames.toSeq
   val keyNames: Seq[String] = List(v1, v2)
   
   scala> val keyIndex = keyNames.zipWithIndex.find(_._1.equalsIgnoreCase("v1")).unzip._2
   val keyIndex: Option[Int] = Some(0)
   
   scala> val value = keyIndex.map(c1Instance.productElement)
   val value: Option[Any] = Some(this is v1)
   
   scala> 
   
   ```



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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r891237351


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/submit/SubmitBatchCommand.scala:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.submit
+
+import java.util.{ArrayList, HashMap}
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.{BatchRestApi, KyuubiRestClient}
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest, OperationLog}
+import org.apache.kyuubi.ctl.{CliConfig, ClientFactory}
+import org.apache.kyuubi.ctl.cmd.Command
+
+class SubmitBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  private var map: HashMap[String, Object] = null
+
+  override def validateArguments(): Unit = {
+    map = readConfig()
+  }
+
+  override def run(): Unit = {
+    val kyuubiRestClient: KyuubiRestClient = ClientFactory.getKyuubiRestClient(cliArgs, map, conf)
+    val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+    val request = map.get("request").asInstanceOf[HashMap[String, Object]]
+    val batchRequest = new BatchRequest(
+      map.get("batchType").asInstanceOf[String],
+      request.get("resource").asInstanceOf[String],
+      request.get("className").asInstanceOf[String],
+      request.get("name").asInstanceOf[String],
+      request.get("config").asInstanceOf[HashMap[String, String]],
+      request.get("args").asInstanceOf[ArrayList[String]])
+
+    var batch: Batch = batchRestApi.createBatch(batchRequest)
+    val batchId = batch.getId
+    var log: OperationLog = null
+    var done = false
+    while (!done) {
+      log = batchRestApi.getBatchLocalLog(
+        batchId,
+        -1,
+        20)
+      log.getLogRowSet.asScala.foreach(x => info(x))
+
+      batch = batchRestApi.getBatchById(batchId)
+      if (log.getLogRowSet.size() == 0 && batch.getState() != "RUNNING") {

Review Comment:
   log.getLogRowSet.size() == 0 && batch.getState() != "PENDING" && batch.getState() != "RUNNING"



##########
kyuubi-ctl/src/test/resources/cli/batch.yaml:
##########
@@ -0,0 +1,32 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+apiVersion: v1
+user: test_user
+request:
+  name: test_batch
+  resource: /MySpace/kyuubi-spark-sql-engine_2.12-1.6.0-SNAPSHOT.jar
+  className: org.apache.kyuubi.engine.spark.SparkSQLEngine
+  args:
+    - x1
+    - x2
+  configs:
+    hive.server2.proxy.user: b_user
+    wait.completion: true
+    k1: v1
+options:
+  verbose: true

Review Comment:
   new line at end of file



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


[GitHub] [incubator-kyuubi] turboFei commented on pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#issuecomment-1154594295

   ```
   BatchCliArgumentsSuite:
   - create/submit batch
   - create/submit batch and overwrite rest config
   - create/submit batch without filename specified
   - create/submit batch with non-existed file
   - get batch without batch id specified
   - get batch
   - test list batch option
   - test list batch default option *** FAILED ***
     BatchOpts(null,null,null,null,0,0,-1,100,null) had size 100 instead of expected size 10 (BatchCliArgumentsSuite.scala:126)
   - test bad list batch option - size
   - test bad list batch option - create date format
   - test bad list batch option - end date format
   - test bad list batch option - negative create date
   - test bad list batch option - negative end date
   - test bad list batch option - createTime > endTime
   - test log batch
   - test log batch without batchId
   - test log batch default option *** FAILED ***
     BatchOpts(f7fd702c-e54e-11ec-8fea-0242ac120002,null,null,null,0,0,-1,100,null) had size 100 instead of expected size 10 (BatchCliArgumentsSuite.scala:250)
   ```


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


[GitHub] [incubator-kyuubi] lightning-L commented on pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
lightning-L commented on PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#issuecomment-1154609997

   > ```
   > BatchCliArgumentsSuite:
   > - create/submit batch
   > - create/submit batch and overwrite rest config
   > - create/submit batch without filename specified
   > - create/submit batch with non-existed file
   > - get batch without batch id specified
   > - get batch
   > - test list batch option
   > - test list batch default option *** FAILED ***
   >   BatchOpts(null,null,null,null,0,0,-1,100,null) had size 100 instead of expected size 10 (BatchCliArgumentsSuite.scala:126)
   > - test bad list batch option - size
   > - test bad list batch option - create date format
   > - test bad list batch option - end date format
   > - test bad list batch option - negative create date
   > - test bad list batch option - negative end date
   > - test bad list batch option - createTime > endTime
   > - test log batch
   > - test log batch without batchId
   > - test log batch default option *** FAILED ***
   >   BatchOpts(f7fd702c-e54e-11ec-8fea-0242ac120002,null,null,null,0,0,-1,100,null) had size 100 instead of expected size 10 (BatchCliArgumentsSuite.scala:250)
   > ```
   
   fixed....


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


[GitHub] [incubator-kyuubi] turboFei closed pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei closed pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation
URL: https://github.com/apache/incubator-kyuubi/pull/2823


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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895147477


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/CommandLine.scala:
##########
@@ -137,4 +194,94 @@ object CommandLine {
           .text("The engine share level this engine belong to."))
   }
 
+  private def batchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+  }
+
+  private def getBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."))
+  }
+
+  private def deleteBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."),
+        opt[String]("hs2ProxyUser")
+          .action((v, c) => c.copy(createOpts = c.createOpts.copy(filename = v)))
+          .text("The value of hive.server2.proxy.user config."))
+  }
+
+  private def listBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        opt[String]("batchType")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchType = v)))
+          .text("Batch type."),
+        opt[String]("batchUser")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchUser = v)))
+          .text("Batch user."),
+        opt[String]("batchState")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchState = v)))
+          .text("Batch state."),
+        opt[String]("createTime")
+          .action((v, c) =>
+            c.copy(batchOpts = c.batchOpts.copy(createTime =
+              dateStringToMillis(v, "yyyyMMddHHmmss"))))
+          .validate(x =>
+            if (x.matches("\\d{14}")) success
+            else failure("Option --createTime must be in yyyyMMddHHmmss format."))
+          .text("Batch create time, should be in yyyyMMddHHmmss format."),
+        opt[String]("endTime")
+          .action((v, c) =>
+            c.copy(batchOpts = c.batchOpts.copy(endTime =
+              dateStringToMillis(v, "yyyyMMddHHmmss"))))
+          .validate(x =>
+            if (x.matches("\\d{14}")) success
+            else failure("Option --endTime must be in yyyyMMddHHmmss format."))
+          .text("Batch end time, should be in yyyyMMddHHmmss format."),
+        opt[Int]("from")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(from = v)))
+          .validate(x =>
+            if (x >= 0) success
+            else failure("Option --from must be >=0"))
+          .text("Specify which record to start from retrieving info."),
+        opt[Int]("size")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(size = v)))
+          .validate(x =>
+            if (x >= 0) success
+            else failure("Option --size must be >=0"))
+          .text("The max number of records returned in the query."))
+  }
+
+  private def logBatchCmd(builder: OParserBuilder[CliConfig]): OParser[_, CliConfig] = {
+    import builder._
+    cmd("batch").action((_, c) => c.copy(resource = ControlObject.BATCH))
+      .children(
+        arg[String]("<batchId>")
+          .optional()
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(batchId = v)))
+          .text("Batch id."),
+        opt[Int]("from")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(from = v)))
+          .text("Specify which record to start from retrieving info."),
+        opt[Int]("size")
+          .action((v, c) => c.copy(batchOpts = c.batchOpts.copy(size = v)))
+          .validate(x =>
+            if (x >= 0) success
+            else failure("Option --size must be >=0"))

Review Comment:
   ditto



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


[GitHub] [incubator-kyuubi] lightning-L commented on pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
lightning-L commented on PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#issuecomment-1154670161

   > please followup below comments in other prs. @lightning-L
   > 
   > 1. support `waitUntilComplete` for submit batch(wait the batch state be running)
   > 2. for log batch/submit batch, call the kyuubi instance that created batch first, failover to original url if failed, it is to reduce the internal rest requests redirection and jdbc state store calls
   
   ok


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


[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2823: [KYUUBI #2628][SUB-TASK][KPIP-4] Implement kyuubi-ctl for batch job operation

Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2823:
URL: https://github.com/apache/incubator-kyuubi/pull/2823#discussion_r895330714


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/submit/SubmitBatchCommand.scala:
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kyuubi.ctl.cmd.submit
+
+import java.util.{ArrayList, HashMap}
+
+import scala.collection.JavaConverters._
+
+import org.apache.kyuubi.client.BatchRestApi
+import org.apache.kyuubi.client.api.v1.dto.{Batch, BatchRequest, OperationLog}
+import org.apache.kyuubi.ctl.CliConfig
+import org.apache.kyuubi.ctl.RestClientFactory.withKyuubiRestClient
+import org.apache.kyuubi.ctl.cmd.Command
+import org.apache.kyuubi.ctl.util.{CtlUtils, Validator}
+
+class SubmitBatchCommand(cliConfig: CliConfig) extends Command(cliConfig) {
+
+  def validate(): Unit = {
+    Validator.validateFilename(normalizedCliConfig)
+  }
+
+  def run(): Unit = {
+    val map = CtlUtils.loadYamlAsMap(normalizedCliConfig)
+
+    withKyuubiRestClient(normalizedCliConfig, map, conf) { kyuubiRestClient =>
+      val batchRestApi: BatchRestApi = new BatchRestApi(kyuubiRestClient)
+
+      val request = map.get("request").asInstanceOf[HashMap[String, Object]]
+      val batchRequest = new BatchRequest(
+        map.get("batchType").asInstanceOf[String],
+        request.get("resource").asInstanceOf[String],
+        request.get("className").asInstanceOf[String],
+        request.get("name").asInstanceOf[String],
+        request.get("configs").asInstanceOf[HashMap[String, String]],
+        request.get("args").asInstanceOf[ArrayList[String]])
+
+      var batch: Batch = batchRestApi.createBatch(batchRequest)
+      val batchId = batch.getId
+      var log: OperationLog = null
+      var done = false
+      var from = 0
+      val size = 20
+      while (!done) {

Review Comment:
   how about do it in another pr?
   
   This pr is too huge.



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