zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zjf...@apache.org
Subject zeppelin git commit: [ZEPPELIN-3014] NPE bug fix and Error message enhancement with Kylin Interpreter
Date Wed, 13 Dec 2017 10:54:12 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/master 3505625c2 -> 549bce673


[ZEPPELIN-3014] NPE bug fix and Error message enhancement with Kylin Interpreter

### What is this PR for?
A few sentences describing the overall goals of the pull request's commits.
First time? Check out the contributing guide - https://zeppelin.apache.org/contribution/contributions.html

### What type of PR is it?
Bug Fix

### Todos
* [ ] - Task

### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3014

### How should this be tested?
*  Setup Travis CI as described on https://zeppelin.apache.org/contribution/contributions.html#continuous-integration
* Use existing unit tests in kylin module.

### Screenshots (if appropriate)

#### before: NPE when result set is empty
![image](https://user-images.githubusercontent.com/18542573/32154048-f1b8ba58-bcfb-11e7-98cc-98cdf484f2d5.png)
#### after: no NPE when result set is empty, just an empty table
![image](https://user-images.githubusercontent.com/18542573/32154069-110215d0-bcfc-11e7-87e9-cc049001f1c7.png)

#### before: when query fails, only error code is returned, no error message
![image](https://user-images.githubusercontent.com/18542573/32154088-29651938-bcfc-11e7-9e66-cd2cfccba054.png)
#### after: when query fails, both error code and error message are displayed to users
![image](https://user-images.githubusercontent.com/18542573/32154096-3d3ab01c-bcfc-11e7-8cf3-d710d96b8c5a.png)

### Questions:
* Does the licenses files need update? No.
* Is there breaking changes for older versions? No.
* Does this needs documentation? No.

Author: Liu <jinxliu@ebay.com>

Closes #2645 from jinxliu/kylin-intp-new and squashes the following commits:

d5692bf [Liu] refactor
85b6424 [Liu] add test for empty result set
4596470 [Liu] ZEPPELIN-3014: NPE bug fix and Error message enhancement with Kylin Interpreter


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/549bce67
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/549bce67
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/549bce67

Branch: refs/heads/master
Commit: 549bce6738ffd7f460867d3f5ee00a9e2ec14125
Parents: 3505625
Author: Liu <jinxliu@ebay.com>
Authored: Tue Dec 5 15:24:20 2017 +0800
Committer: Jeff Zhang <zjffdu@apache.org>
Committed: Wed Dec 13 18:54:06 2017 +0800

----------------------------------------------------------------------
 .../zeppelin/kylin/KylinErrorResponse.java      | 63 +++++++++++++++++
 .../apache/zeppelin/kylin/KylinInterpreter.java | 71 ++++++++++++--------
 .../zeppelin/kylin/KylinInterpreterTest.java    | 24 +++++++
 3 files changed, 130 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/549bce67/kylin/src/main/java/org/apache/zeppelin/kylin/KylinErrorResponse.java
----------------------------------------------------------------------
diff --git a/kylin/src/main/java/org/apache/zeppelin/kylin/KylinErrorResponse.java b/kylin/src/main/java/org/apache/zeppelin/kylin/KylinErrorResponse.java
new file mode 100644
index 0000000..00439e8
--- /dev/null
+++ b/kylin/src/main/java/org/apache/zeppelin/kylin/KylinErrorResponse.java
@@ -0,0 +1,63 @@
+/*
+ * 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.zeppelin.kylin;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonSyntaxException;
+import org.apache.zeppelin.common.JsonSerializable;
+
+/**
+ * class for Kylin Error Response.
+ */
+class KylinErrorResponse implements JsonSerializable {
+  private static final Gson gson = new Gson();
+
+  private String stacktrace;
+  private String exception;
+  private String url;
+  private String code;
+  private Object data;
+  private String msg;
+
+  public KylinErrorResponse(String stacktrace, String exception, String url,
+      String code, Object data, String msg) {
+    this.stacktrace = stacktrace;
+    this.exception = exception;
+    this.url = url;
+    this.code = code;
+    this.data = data;
+    this.msg = msg;
+  }
+
+  public String getException() {
+    return exception;
+  }
+
+  public String toJson() {
+    return gson.toJson(this);
+  }
+
+  public static KylinErrorResponse fromJson(String json) {
+    try {
+      return gson.fromJson(json, KylinErrorResponse.class);
+    } catch (JsonSyntaxException ex) {
+      return null;
+    }
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/549bce67/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java
----------------------------------------------------------------------
diff --git a/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java b/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java
index 6b68d28..c7cd689 100755
--- a/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java
+++ b/kylin/src/main/java/org/apache/zeppelin/kylin/KylinInterpreter.java
@@ -18,6 +18,7 @@
 package org.apache.zeppelin.kylin;
 
 import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpPost;
@@ -30,9 +31,7 @@ import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.util.List;
 import java.util.Properties;
 import java.util.regex.Matcher;
@@ -166,28 +165,42 @@ public class KylinInterpreter extends Interpreter {
   }
 
   private InterpreterResult executeQuery(String sql) throws IOException {
-
     HttpResponse response = prepareRequest(sql);
+    String result;
 
-    if (response.getStatusLine().getStatusCode() != 200) {
-      logger.error("failed to execute query: " + response.getEntity().getContent().toString());
-      return new InterpreterResult(InterpreterResult.Code.ERROR,
-          "Failed : HTTP error code " + response.getStatusLine().getStatusCode());
-    }
-
-    BufferedReader br = new BufferedReader(
-        new InputStreamReader((response.getEntity().getContent())));
-    StringBuilder sb = new StringBuilder();
+    try {
+      int code = response.getStatusLine().getStatusCode();
+      result = IOUtils.toString(response.getEntity().getContent(), "UTF-8");
+
+      if (code != 200) {
+        StringBuilder errorMessage = new StringBuilder("Failed : HTTP error code " + code
+ " .");
+        logger.error("Failed to execute query: " + result);
+
+        KylinErrorResponse kylinErrorResponse = KylinErrorResponse.fromJson(result);
+        if (kylinErrorResponse == null) {
+          logger.error("Cannot get json from string: " + result);
+          // when code is 401, the response is html, not json
+          if (code == 401) {
+            errorMessage.append(" Error message: Unauthorized. This request requires "
+                + "HTTP authentication. Please make sure your have set your credentials "
+                + "correctly.");
+          } else {
+            errorMessage.append(" Error message: " + result + " .");
+          }
+        } else {
+          String exception = kylinErrorResponse.getException();
+          logger.error("The exception is " + exception);
+          errorMessage.append(" Error message: " + exception + " .");
+        }
 
-    String output;
-    logger.info("Output from Server .... \n");
-    while ((output = br.readLine()) != null) {
-      logger.info(output);
-      sb.append(output).append('\n');
+        return new InterpreterResult(InterpreterResult.Code.ERROR, errorMessage.toString());
+      }
+    } catch (NullPointerException | IOException e) {
+      throw new IOException(e);
     }
-    InterpreterResult rett = new InterpreterResult(InterpreterResult.Code.SUCCESS, 
-        formatResult(sb.toString()));
-    return rett;
+
+    return new InterpreterResult(InterpreterResult.Code.SUCCESS,
+        formatResult(result));
   }
 
   String formatResult(String msg) {
@@ -205,16 +218,18 @@ public class KylinInterpreter extends Interpreter {
       table = mr.group(1);
     }
 
-    String[] row = table.split("],\\[");
-    for (int i = 0; i < row.length; i++) {
-      String[] col = row[i].split(",(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)", -1);
-      for (int j = 0; j < col.length; j++) {
-        if (col[j] != null) {
-          col[j] = col[j].replaceAll("^\"|\"$", "");
+    if (table != null && !table.isEmpty()) {
+      String[] row = table.split("],\\[");
+      for (int i = 0; i < row.length; i++) {
+        String[] col = row[i].split(",(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)", -1);
+        for (int j = 0; j < col.length; j++) {
+          if (col[j] != null) {
+            col[j] = col[j].replaceAll("^\"|\"$", "");
+          }
+          res.append(col[j] + " \t");
         }
-        res.append(col[j] + " \t");
+        res.append(" \n");
       }
-      res.append(" \n");
     }
     return res.toString();
   }

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/549bce67/kylin/src/test/java/org/apache/zeppelin/kylin/KylinInterpreterTest.java
----------------------------------------------------------------------
diff --git a/kylin/src/test/java/org/apache/zeppelin/kylin/KylinInterpreterTest.java b/kylin/src/test/java/org/apache/zeppelin/kylin/KylinInterpreterTest.java
index 4471a07..35f0f3c 100755
--- a/kylin/src/test/java/org/apache/zeppelin/kylin/KylinInterpreterTest.java
+++ b/kylin/src/test/java/org/apache/zeppelin/kylin/KylinInterpreterTest.java
@@ -108,6 +108,30 @@ public class KylinInterpreterTest {
     Assert.assertEquals(expected, actual);
   }
 
+  @Test
+  public void testParseEmptyResult() {
+    String msg = "{\"columnMetas\":[{\"isNullable\":1,\"displaySize\":256,\"label\":\"COUNTRY\",\"name\":\"COUNTRY\","
+        + "\"schemaName\":\"DEFAULT\",\"catelogName\":null,\"tableName\":\"SALES_TABLE\",\"precision\":256,"
+        + "\"scale\":0,\"columnType\":12,\"columnTypeName\":\"VARCHAR\",\"writable\":false,\"readOnly\":true,"
+        + "\"definitelyWritable\":false,\"autoIncrement\":false,\"caseSensitive\":true,\"searchable\":false,"
+        + "\"currency\":false,\"signed\":true},{\"isNullable\":1,\"displaySize\":256,\"label\":\"CURRENCY\","
+        + "\"name\":\"CURRENCY\",\"schemaName\":\"DEFAULT\",\"catelogName\":null,\"tableName\":\"SALES_TABLE\","
+        + "\"precision\":256,\"scale\":0,\"columnType\":12,\"columnTypeName\":\"VARCHAR\",\"writable\":false,"
+        + "\"readOnly\":true,\"definitelyWritable\":false,\"autoIncrement\":false,\"caseSensitive\":true,"
+        + "\"searchable\":false,\"currency\":false,\"signed\":true},{\"isNullable\":0,\"displaySize\":19,"
+        + "\"label\":\"COUNT__\",\"name\":\"COUNT__\",\"schemaName\":\"DEFAULT\",\"catelogName\":null,"
+        + "\"tableName\":\"SALES_TABLE\",\"precision\":19,\"scale\":0,\"columnType\":-5,\"columnTypeName\":"
+        + "\"BIGINT\",\"writable\":false,\"readOnly\":true,\"definitelyWritable\":false,\"autoIncrement\":false,"
+        + "\"caseSensitive\":true,\"searchable\":false,\"currency\":false,\"signed\":true}],\"results\":"
+        + "[]," + "\"cube\":\"Sample_Cube\",\"affectedRowCount\":0,\"isException\":false,\"exceptionMessage\":null,"
+        + "\"duration\":134,\"totalScanCount\":1,\"hitExceptionCache\":false,\"storageCacheUsed\":false,"
+        + "\"partial\":false}";
+    String expected="%table COUNTRY \tCURRENCY \tCOUNT__ \t \n";
+    KylinInterpreter t = new MockKylinInterpreter(getDefaultProperties());
+    String actual = t.formatResult(msg);
+    Assert.assertEquals(expected, actual);
+  }
+
   private Properties getDefaultProperties() {
     Properties prop = new Properties();
     prop.put("kylin.api.username", "ADMIN");


Mime
View raw message