aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKunal Khatua <kkhatua@maprtech.com>2019-02-22 13:41:02 -0800
committerGautam Parai <gparai@apache.org>2019-02-22 23:33:35 -0800
commitc17c59cddcced703c9852038e957147c502d17aa (patch)
tree5b7685a5479394971afe8befd1744b79eb90705a
parent110c3578aa18f596278e251d700c8fa9ada8b0c4 (diff)
DRILL-7036: Improve UI for alert and error messages
closes #1644 This PR standardizes error and alert messages to a cleaner interface by leveraging Bootstraps UX elements for publishing the messages in a presentable format. Exceptions reported back to the browser and rendered in a neat tabular format (using Panels) All errors can be redirected to errorMessage.ftl which will render it in a neat format. Alerts are replaced with modals. Interactions (pages) affected by Alert modals 1. Missing Query submission 2. profile Query Rerun 3. invalid Profile Listing Fetch 4. invalid Option Value for update 5. Missing username/password submission The errorMessage.ftl has been moved to root dir, and unused `error.ftl` was removed
-rw-r--r--exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogsResources.java10
-rw-r--r--exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java4
-rw-r--r--exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java9
-rw-r--r--exec/java-exec/src/main/resources/rest/alertModals.ftl104
-rw-r--r--exec/java-exec/src/main/resources/rest/error.ftl28
-rw-r--r--exec/java-exec/src/main/resources/rest/errorMessage.ftl (renamed from exec/java-exec/src/main/resources/rest/query/errorMessage.ftl)10
-rw-r--r--exec/java-exec/src/main/resources/rest/options.ftl7
-rw-r--r--exec/java-exec/src/main/resources/rest/profile/list.ftl11
-rw-r--r--exec/java-exec/src/main/resources/rest/profile/profile.ftl5
-rw-r--r--exec/java-exec/src/main/resources/rest/query/query.ftl6
-rw-r--r--exec/java-exec/src/main/resources/rest/static/js/querySubmission.js54
11 files changed, 174 insertions, 74 deletions
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogsResources.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogsResources.java
index a4d92f4d5..51cf994d9 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogsResources.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogsResources.java
@@ -56,6 +56,7 @@ import static org.apache.drill.exec.server.rest.auth.DrillUserPrincipal.ADMIN_RO
@Path("/")
@RolesAllowed(ADMIN_ROLE)
public class LogsResources {
+ private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(LogsResources.class);
@Inject DrillRestServer.UserAuthEnabled authEnabled;
@Inject SecurityContext sc;
@@ -96,8 +97,13 @@ public class LogsResources {
@Path("/log/{name}/content")
@Produces(MediaType.TEXT_HTML)
public Viewable getLog(@PathParam("name") String name) throws IOException {
- LogContent content = getLogJSON(name);
- return ViewableWithPermissions.create(authEnabled.get(), "/rest/logs/log.ftl", sc, content);
+ try {
+ LogContent content = getLogJSON(name);
+ return ViewableWithPermissions.create(authEnabled.get(), "/rest/logs/log.ftl", sc, content);
+ } catch (Exception | Error e) {
+ logger.error("Exception was thrown when fetching log {} :\n{}", name, e);
+ return ViewableWithPermissions.create(authEnabled.get(), "/rest/errorMessage.ftl", sc, e);
+ }
}
@GET
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java
index e62d33d7a..d6d9a21f3 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryResources.java
@@ -92,8 +92,8 @@ public class QueryResources {
final String rowsPerPageValuesAsStr = Joiner.on(",").join(rowsPerPageValues);
return ViewableWithPermissions.create(authEnabled.get(), "/rest/query/result.ftl", sc, new TabularResult(result, rowsPerPageValuesAsStr));
} catch (Exception | Error e) {
- logger.error("Query from Web UI Failed", e);
- return ViewableWithPermissions.create(authEnabled.get(), "/rest/query/errorMessage.ftl", sc, e);
+ logger.error("Query from Web UI Failed: {}", e);
+ return ViewableWithPermissions.create(authEnabled.get(), "/rest/errorMessage.ftl", sc, e);
}
}
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
index e88b57cb4..86e3ddeb4 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java
@@ -376,8 +376,13 @@ public class ProfileResources {
@Path("/profiles/{queryid}")
@Produces(MediaType.TEXT_HTML)
public Viewable getProfile(@PathParam("queryid") String queryId){
- ProfileWrapper wrapper = new ProfileWrapper(getQueryProfile(queryId), work.getContext().getConfig());
- return ViewableWithPermissions.create(authEnabled.get(), "/rest/profile/profile.ftl", sc, wrapper);
+ try {
+ ProfileWrapper wrapper = new ProfileWrapper(getQueryProfile(queryId), work.getContext().getConfig());
+ return ViewableWithPermissions.create(authEnabled.get(), "/rest/profile/profile.ftl", sc, wrapper);
+ } catch (Exception | Error e) {
+ logger.error("Exception was thrown when fetching profile {} :\n{}", queryId, e);
+ return ViewableWithPermissions.create(authEnabled.get(), "/rest/errorMessage.ftl", sc, e);
+ }
}
@SuppressWarnings("resource")
diff --git a/exec/java-exec/src/main/resources/rest/alertModals.ftl b/exec/java-exec/src/main/resources/rest/alertModals.ftl
new file mode 100644
index 000000000..202c05259
--- /dev/null
+++ b/exec/java-exec/src/main/resources/rest/alertModals.ftl
@@ -0,0 +1,104 @@
+<#--
+
+ 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.
+
+-->
+<style>
+ .modalHeaderAlert, .closeX {
+ color:red !important;
+ text-align: center;
+ font-size: 16px;
+ }
+</style>
+ <!--
+ Alert Modal to use across templates.
+ By default, modal is hidden and expected to be populated and activated by relevant JavaScripts
+ -->
+ <div class="modal" id="errorModal" role="dialog" data-backdrop="static" data-keyboard="false" style="display: none;" aria-hidden="true">
+ <div class="modal-dialog">
+ <!-- Modal content-->
+ <div class="modal-content">
+ <div class="modal-header modalHeaderAlert">
+ <button type="button" class="close closeX" data-dismiss="modal" style="color:red;font-size:200%">×</button>
+ <h4 class="modal-title"><span class="glyphicon glyphicon-alert" style="font-size:125%"></span><span id="modalHeader" style="font-family:Helvetica Neue,Helvetica,Arial,sans-serif;white-space:pre">~ErrorMessage~ Title</span></h4>
+ </div>
+ <div class="modal-body" id="modalBody" style="line-height:3">
+ ~ErrorMessage Details~
+ </div>
+ <div class="modal-footer">
+ <button type="button" class="btn btn-info btn-default" data-dismiss="modal">Close</button>
+ </div>
+ </div>
+ </div>
+ </div>
+<script>
+ //Populate the alert modal with the right message params and show
+ function populateAndShowAlert(errorMsg, inputValues) {
+ if (!(errorMsg in errorMap)) {
+ //Using default errorId to represent message
+ modalHeader.innerHTML=errorMsg;
+ modalBody.innerHTML="[Auto Description] "+JSON.stringify(inputValues);
+ } else {
+ modalHeader.innerHTML=errorMap[errorMsg].msgHeader;
+ modalBody.innerHTML=errorMap[errorMsg].msgBody;
+ }
+ //Check if substitutions are needed
+ let updatedHtml=modalBody.innerHTML;
+ if (inputValues != null) {
+ var inputValuesKeys = Object.keys(inputValues);
+ for (i=0; i<inputValuesKeys.length; ++i) {
+ let currKey=inputValuesKeys[i];
+ updatedHtml=updatedHtml.replace(currKey, inputValues[currKey]);
+ }
+ modalBody.innerHTML=updatedHtml;
+ }
+ //Show Alert
+ $('#errorModal').modal('show');
+ }
+
+ //Map of error messages to populate the alert modal
+ var errorMap = {
+ "userNameMissing": {
+ msgHeader:" ERROR: Username Needed",
+ msgBody:"Please provide a user name. The field cannot be empty.<br>Username is required since impersonation is enabled"
+ },
+ "passwordMissing": {
+ msgHeader:" ERROR: Password Needed",
+ msgBody:"Please provide a password. The field cannot be empty."
+ },
+ "invalidRowCount": {
+ msgHeader:" ERROR: Invalid RowCount",
+ msgBody:"\"<span style='font-family:courier;white-space:pre'>_autoLimitValue_</span>\" is not a number. Please fill in a valid number.",
+ },
+ "invalidProfileFetchSize": {
+ msgHeader:" ERROR: Invalid Fetch Size",
+ msgBody:"\"<span style='font-family:courier;white-space:pre'>_fetchSize_</span>\" is not a valid fetch size.<br>Please enter a valid number of profiles to fetch (1 to 100000)",
+ },
+ "invalidOptionValue": {
+ msgHeader:" ERROR: Invalid Option Value",
+ msgBody:"\"<span style='font-family:courier;white-space:pre'>_numericOption_</span>\" is not a valid numeric value for <span style='font-family:courier;white-space:pre'>_optionName_</span><br>Please enter a valid number to update the option.",
+ },
+ "queryMissing": {
+ msgHeader:" ERROR: No Query to execute",
+ msgBody:"Please provide a query. The query textbox cannot be empty."
+ },
+ "errorId": {
+ msgHeader:"~header~",
+ msgBody:"Description unavailable"
+ }
+ }
+</script> \ No newline at end of file
diff --git a/exec/java-exec/src/main/resources/rest/error.ftl b/exec/java-exec/src/main/resources/rest/error.ftl
deleted file mode 100644
index 398ae0780..000000000
--- a/exec/java-exec/src/main/resources/rest/error.ftl
+++ /dev/null
@@ -1,28 +0,0 @@
-<#--
-
- 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.
-
--->
-<html>
-
-<pre>
-${model.printStackTrace()}
-</pre>
-
-
-
-<html> \ No newline at end of file
diff --git a/exec/java-exec/src/main/resources/rest/query/errorMessage.ftl b/exec/java-exec/src/main/resources/rest/errorMessage.ftl
index 64662080e..9951d9d72 100644
--- a/exec/java-exec/src/main/resources/rest/query/errorMessage.ftl
+++ b/exec/java-exec/src/main/resources/rest/errorMessage.ftl
@@ -24,8 +24,14 @@
<#macro page_body>
<div class="page-header">
</div>
- <h2> Query Failed: An Error Occurred </h2>
- ${model}
+<!-- Rendering Panel -->
+ <div class="panel panel-danger">
+ <div class="panel-heading" style="white-space:pre;font-size:110%;padding:0px 0px">
+ <span class="glyphicon glyphicon-alert" style="font-size:125%;">&#xe209;</span> <strong>${model.getClass().getSimpleName()} :</strong> ${model.getMessage()?split("\n")[0]}
+ </div>
+ <div class="panel-body"><span style="font-family:courier,monospace;white-space:pre-wrap">${model}</span></div>
+ </div>
+ <a class="btn btn-default" id="backBtn" style="display:inline" onclick="window.history.back()"><span class="glyphicon glyphicon-step-backward"></span> Back</a>
</#macro>
<@page_html/> \ No newline at end of file
diff --git a/exec/java-exec/src/main/resources/rest/options.ftl b/exec/java-exec/src/main/resources/rest/options.ftl
index e1c904d03..9b934e904 100644
--- a/exec/java-exec/src/main/resources/rest/options.ftl
+++ b/exec/java-exec/src/main/resources/rest/options.ftl
@@ -52,7 +52,10 @@
optionValue = $("#"+optionName+" select[name='value']").val();
} else if (optionKind != "STRING") { //i.e. it is a number (FLOAT/DOUBLE/LONG)
if (isNaN(optionValue)) {
- alert(optionValue+" is not a valid number for option: "+optionName);
+ let actualOptionName=optionName.replace(/\\\./gi, ".");
+ let alertValues = {'_numericOption_': optionValue, '_optionName_': actualOptionName };
+ populateAndShowAlert('invalidOptionValue', alertValues);
+ $("#"+optionName+" input[name='value']").focus();
return;
}
}
@@ -84,7 +87,7 @@
<button type="button" class="btn btn-info" onclick="inject(this.innerHTML);">${filter}</button>
</#list>
</div>
-
+ <#include "*/alertModals.ftl">
<div class="table-responsive">
<table id='optionsTbl' class="table table-striped table-condensed display sortable" style="table-layout: auto; width=100%;">
<thead>
diff --git a/exec/java-exec/src/main/resources/rest/profile/list.ftl b/exec/java-exec/src/main/resources/rest/profile/list.ftl
index 1afeb7deb..5e134189f 100644
--- a/exec/java-exec/src/main/resources/rest/profile/list.ftl
+++ b/exec/java-exec/src/main/resources/rest/profile/list.ftl
@@ -69,8 +69,6 @@
//Submit Cancellations & show status
function cancelSelection() {
let checkedBoxes = document.querySelectorAll('input[name=cancelQ]:checked');
- //dBug
- console.log("Cancelling => " + checkedBoxes.length);
let checkedCount = checkedBoxes.length;
if (checkedCount == 0) return;
@@ -154,15 +152,18 @@
<strong>No running queries.</strong>
</div>
</#if>
+
+ <#include "*/alertModals.ftl">
+
<table width="100%">
<script type="text/javascript" language="javascript">
//Validate that the fetch number is valid
function checkMaxFetch() {
var maxFetch = document.forms["profileFetch"]["max"].value;
- console.log("maxFetch: " + maxFetch);
if (isNaN(maxFetch) || (maxFetch < 1) || (maxFetch > 100000) ) {
- alert("Invalid Entry: " + maxFetch + "\n" +
- "Please enter a valid number of profiles to fetch (1 to 100000) ");
+ let alertValues = {'_fetchSize_': maxFetch };
+ populateAndShowAlert('invalidProfileFetchSize', alertValues);
+ $("#fetchMax").focus();
return false;
}
return true;
diff --git a/exec/java-exec/src/main/resources/rest/profile/profile.ftl b/exec/java-exec/src/main/resources/rest/profile/profile.ftl
index 6d5690a06..d34bbf4d5 100644
--- a/exec/java-exec/src/main/resources/rest/profile/profile.ftl
+++ b/exec/java-exec/src/main/resources/rest/profile/profile.ftl
@@ -162,12 +162,13 @@
</label>
</div>
</div>
- <button class="btn btn-default" type="button" onclick="<#if model.isOnlyImpersonationEnabled()>doSubmitQueryWithUserName()<#else>submitQuery()</#if>">
+ <button class="btn btn-default" type="button" onclick="<#if model.isOnlyImpersonationEnabled()>doSubmitQueryWithUserName()<#else>doSubmitQueryWithAutoLimit()</#if>">
Re-run query
</button>
</form>
</p>
+<#include "*/alertModals.ftl">
<#include "*/runningQuery.ftl">
<p>
@@ -563,7 +564,7 @@
.addEventListener('keydown', function(e) {
if (!(e.keyCode == 13 && (e.metaKey || e.ctrlKey))) return;
if (e.target.form)
- <#if model.isOnlyImpersonationEnabled()>doSubmitQueryWithUserName()<#else>submitQuery()</#if>;
+ <#if model.isOnlyImpersonationEnabled()>doSubmitQueryWithUserName()<#else>doSubmitQueryWithAutoLimit()</#if>;
});
</script>
diff --git a/exec/java-exec/src/main/resources/rest/query/query.ftl b/exec/java-exec/src/main/resources/rest/query/query.ftl
index c4549f7a5..c1a4734b7 100644
--- a/exec/java-exec/src/main/resources/rest/query/query.ftl
+++ b/exec/java-exec/src/main/resources/rest/query/query.ftl
@@ -39,6 +39,8 @@
Sample SQL query: <strong>SELECT * FROM cp.`employee.json` LIMIT 20</strong>
</div>
+<#include "*/alertModals.ftl">
+
<#include "*/runningQuery.ftl">
<#if model.isOnlyImpersonationEnabled()>
@@ -77,7 +79,7 @@
<input class="form-control" type="hidden" id="query" name="query"/>
</div>
- <button class="btn btn-default" type="button" onclick="<#if model.isOnlyImpersonationEnabled()>doSubmitQueryWithUserName()<#else>wrapAndSubmitQuery()</#if>">
+ <button class="btn btn-default" type="button" onclick="<#if model.isOnlyImpersonationEnabled()>doSubmitQueryWithUserName()<#else>doSubmitQueryWithAutoLimit()</#if>">
Submit
</button>
<input type="checkbox" name="forceLimit" value="limit" <#if model.isAutoLimitEnabled()>checked</#if>> Limit results to <input type="text" id="queryLimit" min="0" value="${model.getDefaultRowsAutoLimited()}" size="6" pattern="[0-9]*"> rows <span class="glyphicon glyphicon-info-sign" onclick="alert('Limits the number of records retrieved in the query')" style="cursor:pointer"></span>
@@ -127,7 +129,7 @@
.addEventListener('keydown', function(e) {
if (!(e.keyCode == 13 && (e.metaKey || e.ctrlKey))) return;
if (e.target.form) //Submit [Wrapped] Query
- <#if model.isOnlyImpersonationEnabled()>doSubmitQueryWithUserName()<#else>wrapAndSubmitQuery()</#if>;
+ <#if model.isOnlyImpersonationEnabled()>doSubmitQueryWithUserName()<#else>doSubmitQueryWithAutoLimit()</#if>;
});
</script>
</#macro>
diff --git a/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js b/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js
index 1183682a7..a3cc8bce1 100644
--- a/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js
+++ b/exec/java-exec/src/main/resources/rest/static/js/querySubmission.js
@@ -43,17 +43,39 @@ function closePopup() {
function doSubmitQueryWithUserName() {
userName = document.getElementById("userName").value;
if (!userName.trim()) {
- alert("Please fill in User Name field");
+ populateAndShowAlert('userNameMissing', null);
+ $("#userName").focus();
return;
}
//Wrap and Submit query
- wrapAndSubmitQuery();
+ doSubmitQueryWithAutoLimit();
}
-//Wrap & Submit Query (invoked directly if impersonation is not enabled)
-function wrapAndSubmitQuery() {
+//Perform autoLimit check before submitting (invoked directly if impersonation is not enabled)
+function doSubmitQueryWithAutoLimit() {
+ let origQueryText = $('#query').attr('value');
+ if (origQueryText == null || origQueryText.trim().length == 0) {
+ populateAndShowAlert("queryMissing", null);
+ $("#query").focus();
+ return;
+ }
//Wrap if required
- wrapQuery();
+ let mustWrapWithLimit = $('input[name="forceLimit"]:checked').length > 0;
+ //Clear field when submitting if not mustWrapWithLimit
+ if (!mustWrapWithLimit) {
+ //Wipe out any numeric entry in the field before
+ $('#queryLimit').attr('value', '');
+ } else {
+ let autoLimitValue=document.getElementById('queryLimit').value;
+ let positiveIntRegex = new RegExp("^[1-9]\\d*$");
+ let isValidRowCount = positiveIntRegex.test(autoLimitValue);
+ if (!isValidRowCount) {
+ let alertValues = {'_autoLimitValue_': autoLimitValue };
+ populateAndShowAlert("invalidRowCount", alertValues);
+ $('#queryLimit').focus();
+ return;
+ }
+ }
//Submit query
submitQuery();
}
@@ -83,25 +105,3 @@ function submitQuery() {
}
});
}
-
-//Wraps a query with Limit by directly changing the query in the hidden textbox in the UI (see /query.ftl)
-function wrapQuery() {
- var origQueryText = $('#query').attr('value');
- //dBug: console.log("Query Input:" + origQueryText);
- var mustWrapWithLimit = $('input[name="forceLimit"]:checked').length > 0;
- if (mustWrapWithLimit) {
- var semicolonIdx = origQueryText.lastIndexOf(';');
- //Check and eliminate trailing semicolon
- if (semicolonIdx == origQueryText.length-1 ) {
- origQueryText = origQueryText.substring(0, semicolonIdx)
- }
- var qLimit = $('#queryLimit').val();
- var wrappedQuery = "-- [autoLimit: " + qLimit + " rows]\nselect * from (\n" + origQueryText + "\n) limit " + qLimit;
- //dBug: console.log("Query Output:" + wrappedQuery);
- //Wrapping Query
- $('#query').attr('value', wrappedQuery);
- } else {
- //Do not change the query
- //dBug: console.log("Query Output:" + origQueryText);
- }
-}