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  #### after: no NPE when result set is empty, just an empty table  #### before: when query fails, only error code is returned, no error message  #### after: when query fails, both error code and error message are displayed to users  ### Questions: * Does the licenses files need update? No. * Is there breaking changes for older versions? No. * Does this needs documentation? No. Author: Liu <jinx...@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 <jinx...@ebay.com> Authored: Tue Dec 5 15:24:20 2017 +0800 Committer: Jeff Zhang <zjf...@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");
