[incubator-kyuubi] branch master updated: [KYUUBI #2800][FOLLOWUP] Return CloseBatchReponse for kyuubi rest client deleteBatch

[fe...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new d881d3188 [KYUUBI #2800][FOLLOWUP] Return CloseBatchReponse for kyuubi rest client deleteBatch
d881d3188 is described below

commit d881d3188cb1acad3371d11cc8c57f20ae81a4a3
Author: Fei Wang <fw...@ebay.com>
AuthorDate: Thu Jun 2 23:41:57 2022 +0800

    [KYUUBI #2800][FOLLOWUP] Return CloseBatchReponse for kyuubi rest client deleteBatch
    
    ### _Why are the changes needed?_
    
    Return CloseBatchReponse for kyuubi rest client deleteBatch
    
    ### _How was this patch tested?_
    - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
    
    - [ ] Add screenshots for manual tests if appropriate
    
    - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #2810 from turboFei/delete_followup.
    
    Closes #2800
    
    76e9140d1 [Fei Wang] prevent flaky test
    b10a5e232 [Fei Wang] reforamt
    bb59fa41c [Fei Wang] [KYUUBI #2800][FOLLOWUP] Return CloseBatchReponse for kyuubi rest client deleteBatch
    
    Authored-by: Fei Wang <fw...@ebay.com>
    Signed-off-by: Fei Wang <fw...@ebay.com>
---
 .../src/main/java/org/apache/kyuubi/client/BatchRestApi.java      | 6 +++---
 .../src/main/java/org/apache/kyuubi/client/RestClient.java        | 5 +++++
 .../test/java/org/apache/kyuubi/client/BatchRestClientTest.java   | 7 +++++--
 .../src/test/java/org/apache/kyuubi/client/BatchTestServlet.java  | 5 +++++
 .../test/java/org/apache/kyuubi/client/RestClientTestUtil.java    | 5 +++++
 .../src/main/resources/sql/derby/statestore-schema-derby.sql      | 8 ++++----
 .../org/apache/kyuubi/server/rest/client/BatchRestApiSuite.scala  | 6 ++++--
 7 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/BatchRestApi.java b/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/BatchRestApi.java
index 94a7287b6..76f016d92 100644
--- a/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/BatchRestApi.java
+++ b/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/BatchRestApi.java
@@ -21,6 +21,7 @@ import java.util.HashMap;
 import java.util.Map;
 import org.apache.kyuubi.client.api.v1.dto.Batch;
 import org.apache.kyuubi.client.api.v1.dto.BatchRequest;
+import org.apache.kyuubi.client.api.v1.dto.CloseBatchResponse;
 import org.apache.kyuubi.client.api.v1.dto.GetBatchesResponse;
 import org.apache.kyuubi.client.api.v1.dto.OperationLog;
 import org.apache.kyuubi.client.util.JsonUtil;
@@ -58,7 +59,6 @@ public class BatchRestApi {
 
   public OperationLog getBatchLocalLog(String batchId, int from, int size) {
     Map<String, Object> params = new HashMap<>();
-    params.put("batchId", batchId);
     params.put("from", from);
     params.put("size", size);
 
@@ -66,12 +66,12 @@ public class BatchRestApi {
     return this.getClient().get(path, params, OperationLog.class, client.getAuthHeader());
   }
 
-  public void deleteBatch(String batchId, String hs2ProxyUser) {
+  public CloseBatchResponse deleteBatch(String batchId, String hs2ProxyUser) {
     Map<String, Object> params = new HashMap<>();
     params.put("hive.server2.proxy.user", hs2ProxyUser);
 
     String path = String.format("%s/%s", API_BASE_PATH, batchId);
-    this.getClient().delete(path, params, client.getAuthHeader());
+    return this.getClient().delete(path, params, CloseBatchResponse.class, client.getAuthHeader());
   }
 
   private RestClient getClient() {
diff --git a/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/RestClient.java b/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/RestClient.java
index f6a6fda0f..0e3058d16 100644
--- a/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/RestClient.java
+++ b/kyuubi-rest-client/src/main/java/org/apache/kyuubi/client/RestClient.java
@@ -80,6 +80,11 @@ public class RestClient implements AutoCloseable {
     return doRequest(buildURI(path), authHeader, postRequestBuilder);
   }
 
+  public <T> T delete(String path, Map<String, Object> params, Class<T> type, String authHeader) {
+    String responseJson = delete(path, params, authHeader);
+    return JsonUtil.toObject(responseJson, type);
+  }
+
   public String delete(String path, Map<String, Object> params, String authHeader) {
     return doRequest(buildURI(path, params), authHeader, RequestBuilder.delete());
   }
diff --git a/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchRestClientTest.java b/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchRestClientTest.java
index 3fa05e9f8..8f97f73a9 100644
--- a/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchRestClientTest.java
+++ b/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchRestClientTest.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertEquals;
 
 import org.apache.kyuubi.client.api.v1.dto.Batch;
 import org.apache.kyuubi.client.api.v1.dto.BatchRequest;
+import org.apache.kyuubi.client.api.v1.dto.CloseBatchResponse;
 import org.apache.kyuubi.client.api.v1.dto.GetBatchesResponse;
 import org.apache.kyuubi.client.api.v1.dto.OperationLog;
 import org.apache.kyuubi.client.exception.KyuubiRestException;
@@ -226,11 +227,13 @@ public class BatchRestClientTest {
   public void deleteBatchTest() {
     // test spnego auth
     BatchTestServlet.setAuthSchema(NEGOTIATE_AUTH);
-    spnegoBatchRestApi.deleteBatch("71535", "b_test");
+    CloseBatchResponse response = spnegoBatchRestApi.deleteBatch("71535", "b_test");
+    assertEquals(response.isSuccess(), true);
 
     // test basic auth
     BatchTestServlet.setAuthSchema(BASIC_AUTH);
     BatchTestServlet.allowAnonymous(false);
-    basicBatchRestApi.deleteBatch("71535", "b_test");
+    response = basicBatchRestApi.deleteBatch("71535", "b_test");
+    assertEquals(response.isSuccess(), true);
   }
 }
diff --git a/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchTestServlet.java b/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchTestServlet.java
index 44ab5ec61..f3d4b7292 100644
--- a/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchTestServlet.java
+++ b/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/BatchTestServlet.java
@@ -27,6 +27,7 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.apache.kyuubi.client.api.v1.dto.Batch;
+import org.apache.kyuubi.client.api.v1.dto.CloseBatchResponse;
 import org.apache.kyuubi.client.api.v1.dto.GetBatchesResponse;
 import org.apache.kyuubi.client.api.v1.dto.OperationLog;
 
@@ -103,6 +104,10 @@ public class BatchTestServlet extends HttpServlet {
 
     if (req.getPathInfo().matches("/api/v1/batches/\\d+")) {
       resp.setStatus(HttpServletResponse.SC_OK);
+
+      CloseBatchResponse closeResp = generateTestCloseBatchResp();
+      resp.getWriter().write(MAPPER.writeValueAsString(closeResp));
+      resp.getWriter().flush();
     } else {
       resp.setStatus(HttpServletResponse.SC_NOT_FOUND);
     }
diff --git a/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/RestClientTestUtil.java b/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/RestClientTestUtil.java
index 88d0d5441..440430172 100644
--- a/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/RestClientTestUtil.java
+++ b/kyuubi-rest-client/src/test/java/org/apache/kyuubi/client/RestClientTestUtil.java
@@ -23,6 +23,7 @@ import java.util.List;
 import java.util.Map;
 import org.apache.kyuubi.client.api.v1.dto.Batch;
 import org.apache.kyuubi.client.api.v1.dto.BatchRequest;
+import org.apache.kyuubi.client.api.v1.dto.CloseBatchResponse;
 import org.apache.kyuubi.client.api.v1.dto.GetBatchesResponse;
 import org.apache.kyuubi.client.api.v1.dto.OperationLog;
 
@@ -38,6 +39,10 @@ public class RestClientTestUtil {
     return generateTestBatch("71535");
   }
 
+  public static CloseBatchResponse generateTestCloseBatchResp() {
+    return new CloseBatchResponse(true, "");
+  }
+
   public static Batch generateTestBatch(String id) {
     Map<String, String> batchInfo = new HashMap<>();
     batchInfo.put("id", id);
diff --git a/kyuubi-server/src/main/resources/sql/derby/statestore-schema-derby.sql b/kyuubi-server/src/main/resources/sql/derby/statestore-schema-derby.sql
index 5d38ecd5a..688e8f82f 100644
--- a/kyuubi-server/src/main/resources/sql/derby/statestore-schema-derby.sql
+++ b/kyuubi-server/src/main/resources/sql/derby/statestore-schema-derby.sql
@@ -24,10 +24,10 @@ CREATE TABLE session_metadata(
     end_time bigint  -- the metadata end time
 );
 
-CREATE INDEX metadata_kyuubi_instance_index ON metadata(kyuubi_instance);
+CREATE INDEX metadata_kyuubi_instance_index ON session_metadata(kyuubi_instance);
 
-CREATE INDEX metadata_identifier_index ON metadata(identifier);
+CREATE INDEX metadata_identifier_index ON session_metadata(identifier);
 
-CREATE INDEX metadata_user_name_index ON metadata(user_name);
+CREATE INDEX metadata_user_name_index ON session_metadata(user_name);
 
-CREATE INDEX metadata_engine_type_index ON metadata(engine_type);
+CREATE INDEX metadata_engine_type_index ON session_metadata(engine_type);
diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchRestApiSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchRestApiSuite.scala
index 91edc3b3f..1485c3112 100644
--- a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchRestApiSuite.scala
+++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/rest/client/BatchRestApiSuite.scala
@@ -66,7 +66,8 @@ class BatchRestApiSuite extends RestClientTestHelper {
     assert(log.getRowCount == 1)
 
     // delete batch
-    batchRestApi.deleteBatch(batch.getId(), null)
+    val closeResp = batchRestApi.deleteBatch(batch.getId(), null)
+    assert(closeResp.getMsg.nonEmpty)
 
     basicKyuubiRestClient.close()
   }
@@ -125,7 +126,8 @@ class BatchRestApiSuite extends RestClientTestHelper {
     assert(log.getRowCount == 1)
 
     // delete batch
-    batchRestApi.deleteBatch(batch.getId(), null)
+    val closeResp = batchRestApi.deleteBatch(batch.getId(), null)
+    assert(closeResp.isSuccess)
 
     spnegoKyuubiRestClient.close()
   }