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/28 03:51:45 UTC

[GitHub] [incubator-kyuubi] lightning-L opened a new pull request, #2955: BatchRequest args fix: need toString operation for different data types

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

   <!--
   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 closed pull request #2955: BatchRequest args fix: need toString operation for different data types

Posted by GitBox <gi...@apache.org>.
turboFei closed pull request #2955: BatchRequest args fix: need toString operation for different data types
URL: https://github.com/apache/incubator-kyuubi/pull/2955


-- 
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 #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   yes.
   
   Because it is a filed of dto `BatchRequest`.



-- 
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 #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   BTW, the batchType in https://github.com/apache/incubator-kyuubi/blob/master/kyuubi-ctl/src/test/resources/cli/batch.yaml is missing



-- 
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 #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala

Review Comment:
   ArrayList => JList



-- 
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 #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   should the  batchType from 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 #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   how about read from `map` first, if not defined, then fall back to `request`.



##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   how about reading from `map` first, if not defined, then fall back to `request`?
   
   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] ulysses-you commented on a diff in pull request #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   seems we do not need read from map first, just get it from 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 #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   ```
   request.get("batchType").asInstanceOf[String]
   ```



-- 
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 #2955: BatchRequest args fix: need toString operation for different data types

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

   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955?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 [#2955](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (be40748) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/994dc6ce849f4502e2a9060cc4e5246967eb03d3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (994dc6c) will **decrease** coverage by `0.04%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2955      +/-   ##
   ============================================
   - Coverage     62.89%   62.85%   -0.05%     
     Complexity        6        6              
   ============================================
     Files           418      418              
     Lines         20252    20254       +2     
     Branches       2589     2589              
   ============================================
   - Hits          12738    12731       -7     
   - Misses         6282     6286       +4     
   - Partials       1232     1237       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...che/kyuubi/ctl/cmd/create/CreateBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955/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-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2NyZWF0ZS9DcmVhdGVCYXRjaENvbW1hbmQuc2NhbGE=) | `94.73% <100.00%> (+0.61%)` | :arrow_up: |
   | [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955/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-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `80.00% <0.00%> (-2.00%)` | :arrow_down: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `80.82% <0.00%> (-1.37%)` | :arrow_down: |
   | [...pache/kyuubi/server/metadata/MetadataManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbWV0YWRhdGEvTWV0YWRhdGFNYW5hZ2VyLnNjYWxh) | `82.11% <0.00%> (-1.33%)` | :arrow_down: |
   | [...che/kyuubi/server/KyuubiTHttpFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvS3l1dWJpVEh0dHBGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `63.97% <0.00%> (-0.74%)` | :arrow_down: |
   | [...yuubi/server/metadata/jdbc/JDBCMetadataStore.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbWV0YWRhdGEvamRiYy9KREJDTWV0YWRhdGFTdG9yZS5zY2FsYQ==) | `89.53% <0.00%> (-0.73%)` | :arrow_down: |
   | [.../apache/kyuubi/client/KyuubiSyncThriftClient.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jbGllbnQvS3l1dWJpU3luY1RocmlmdENsaWVudC5zY2FsYQ==) | `87.86% <0.00%> (-0.42%)` | :arrow_down: |
   | [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `97.09% <0.00%> (-0.10%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955?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/2955?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 [994dc6c...be40748](https://codecov.io/gh/apache/incubator-kyuubi/pull/2955?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 pull request #2955: BatchRequest args fix: need toString operation for different data types

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

   thanks, merged 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 #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   ![image](https://user-images.githubusercontent.com/6757692/176089575-bf919bd4-df9c-4420-b1e6-5c49e0a40095.png)
   
   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 #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   the  batchType should from 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] ulysses-you commented on a diff in pull request #2955: BatchRequest args fix: need toString operation for different data types

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


##########
kyuubi-ctl/src/main/scala/org/apache/kyuubi/ctl/cmd/create/CreateBatchCommand.scala:
##########
@@ -43,13 +43,15 @@ class CreateBatchCommand(cliConfig: CliConfig) extends Command[Batch](cliConfig)
       val request = map.get("request").asInstanceOf[JMap[String, Object]]
       val config = request.get("configs").asInstanceOf[JMap[Object, Object]].asScala
         .map { case (k, v) => (k.toString, v.toString) }.asJava
+      val args = request.get("args").asInstanceOf[ArrayList[Object]].asScala
+        .map(x => x.toString).asJava
       val batchRequest = new BatchRequest(
         map.get("batchType").asInstanceOf[String],

Review Comment:
   how can we fall back to request, do you mean  ?
   ```yml
   request:
     name:
     batchType:
   ```



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