This is an automated email from the ASF dual-hosted git repository.
dzamo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push:
new 7105ea3 DRILL-8040: Add HTTP status codes for API errors
7105ea3 is described below
commit 7105ea3b19947ebf53a667e56430ab8d0696559a
Author: James Turton <[email protected]>
AuthorDate: Tue Nov 9 15:38:51 2021 +0200
DRILL-8040: Add HTTP status codes for API errors
---
.../drill/exec/server/rest/QueryResources.java | 25 +++--
.../drill/exec/server/rest/StorageResources.java | 103 +++++++++++++--------
.../exec/server/rest/profile/ProfileResources.java | 26 ++++--
.../main/resources/rest/static/js/serverMessage.js | 4 +-
.../src/main/resources/rest/storage/list.ftl | 16 +++-
.../src/main/resources/rest/storage/update.ftl | 13 ++-
6 files changed, 118 insertions(+), 69 deletions(-)
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 93d2d11..e49431f 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
@@ -98,19 +98,18 @@ public class QueryResources {
@Produces(MediaType.APPLICATION_JSON)
public StreamingOutput submitQueryJSON(QueryWrapper query) throws Exception {
- // Prior to Drill 1.18, REST queries would batch the entire result set
- // in memory, limiting query size. In Drill 1.19 and later, results are
- // streamed from the executor, to the JSON writer and to the HTTP
connection
- // with no buffering.
- //
- // For compatibility with Drill 1.18 and before, Drill places the query
- // schema *after* the data. Any tool that needs to know the schema will
- // need to buffer the entire result set to wait for the schema. An obvious
- // improvement is to provide an option to send the schema *before* the
data.
- // One drawback of doing so is that the schema will report that of the
first
- // batch: Drill allows schema to change across batches and thus the schema
- // of the JSON-encoded data would change. This is more a bug with how Drill
- // handles schemas than a JSON issue. (ODBC and JDBC have the same issues.)
+ /*
+ Prior to Drill 1.18, REST queries would batch the entire result set in
memory,
+ limiting query size. In Drill 1.19 and later, results are streamed from the
+ executor, to the JSON writer and to the HTTP connection with no buffering.
+
+ Starting with Drill 1.19, the "metadata" property specifying the result
column
+ data types is placed *before* the data itself. One drawback of doing so is
that
+ the schema will report that of the first batch: Drill allows schema to
change
+ across batches and thus the schema of the JSON-encoded data would change.
This
+ is more a bug with how Drill handles schemas than a JSON issue. (ODBC and
JDBC
+ have the same issues.)
+ */
QueryRunner runner = new QueryRunner(work, webUserConnection);
try {
runner.start(query);
diff --git
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java
index 0a8e66a..a87cb73 100644
---
a/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java
+++
b/exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/StorageResources.java
@@ -101,15 +101,14 @@ public class StorageResources {
public Response getConfigsFor(@PathParam("group") String pluginGroup,
@PathParam("format") String format) {
format = StringUtils.isNotEmpty(format) ? format.replace("/", "") :
JSON_FORMAT;
return isSupported(format)
- ? Response.ok()
- .entity(getConfigsFor(pluginGroup).toArray())
- .header(HttpHeaders.CONTENT_DISPOSITION,
String.format("attachment;filename=\"%s_storage_plugins.%s\"",
- pluginGroup, format))
- .build()
- : Response.status(Response.Status.NOT_FOUND.getStatusCode())
- .entity(String.format("Unknown \"%s\" file format for \"%s\"
Storage Plugin configs group",
- format, pluginGroup))
- .build();
+ ? Response.ok()
+ .entity(getConfigsFor(pluginGroup).toArray())
+ .header(HttpHeaders.CONTENT_DISPOSITION,
String.format("attachment;filename=\"%s_storage_plugins.%s\"",
+ pluginGroup, format))
+ .build()
+ : Response.status(Response.Status.NOT_ACCEPTABLE)
+ .entity(message("Unknown \"%s\" file format for Storage Plugin
config", format))
+ .build();
}
@GET
@@ -136,20 +135,27 @@ public class StorageResources {
@GET
@Path("/storage/{name}.json")
@Produces(MediaType.APPLICATION_JSON)
- public PluginConfigWrapper getPluginConfig(@PathParam("name") String name) {
+ public Response getPluginConfig(@PathParam("name") String name) {
try {
- return new PluginConfigWrapper(name, storage.getStoredConfig(name));
+ return Response.ok(new PluginConfigWrapper(name,
storage.getStoredConfig(name)))
+ .build();
} catch (Exception e) {
logger.error("Failure while trying to access storage config: {}", name,
e);
+
+ return Response.status(Response.Status.NOT_FOUND)
+ .entity(message("Failure while trying to access storage config: %s",
e.getMessage()))
+ .build();
}
- return new PluginConfigWrapper(name, null);
}
@GET
@Path("/storage/{name}")
@Produces(MediaType.TEXT_HTML)
public Viewable getPlugin(@PathParam("name") String name) {
- StoragePluginModel model = new StoragePluginModel(getPluginConfig(name),
request);
+ StoragePluginModel model = new StoragePluginModel(
+ (PluginConfigWrapper) getPluginConfig(name).getEntity(),
+ request
+ );
return ViewableWithPermissions.create(authEnabled.get(),
"/rest/storage/update.ftl", sc,
model);
}
@@ -158,15 +164,19 @@ public class StorageResources {
@Path("/storage/{name}/enable/{val}")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
- public JsonResult enablePlugin(@PathParam("name") String name,
@PathParam("val") Boolean enable) {
+ public Response enablePlugin(@PathParam("name") String name,
@PathParam("val") Boolean enable) {
try {
storage.setEnabled(name, enable);
- return message("Success");
+ return Response.ok().entity(message("Success")).build();
} catch (PluginNotFoundException e) {
- return message("No plugin exists with the given name: " + name);
+ return Response.status(Response.Status.NOT_FOUND)
+ .entity(message("No plugin exists with the given name: " + name))
+ .build();
} catch (PluginException e) {
- logger.debug("Error in enabling storage name: {} flag: {}", name,
enable);
- return message("Unable to enable/disable plugin:" + e.getMessage());
+ logger.info("Error when enabling storage name: {} flag: {}", name,
enable);
+ return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
+ .entity(message("Unable to enable/disable plugin: %s", e.getMessage()))
+ .build();
}
}
@@ -177,7 +187,7 @@ public class StorageResources {
@Path("/storage/{name}/enable/{val}")
@Produces(MediaType.APPLICATION_JSON)
@Deprecated
- public JsonResult enablePluginViaGet(@PathParam("name") String name,
@PathParam("val") Boolean enable) {
+ public Response enablePluginViaGet(@PathParam("name") String name,
@PathParam("val") Boolean enable) {
return enablePlugin(name, enable);
}
@@ -192,24 +202,28 @@ public class StorageResources {
@Produces(MediaType.APPLICATION_JSON)
public Response exportPlugin(@PathParam("name") String name,
@PathParam("format") String format) {
format = StringUtils.isNotEmpty(format) ? format.replace("/", "") :
JSON_FORMAT;
- return isSupported(format)
- ? Response.ok(getPluginConfig(name))
- .header(HttpHeaders.CONTENT_DISPOSITION,
String.format("attachment;filename=\"%s.%s\"", name, format))
- .build()
- : Response.status(Response.Status.NOT_FOUND.getStatusCode())
- .entity(String.format("Unknown \"%s\" file format for \"%s\"
Storage Plugin config", format, name))
- .build();
+ if (!isSupported(format)) {
+ return Response.status(Response.Status.NOT_ACCEPTABLE)
+ .entity(message("Unknown \"%s\" file format for Storage Plugin
config", format))
+ .build();
+ }
+
+ return Response.ok(getPluginConfig(name))
+ .header(HttpHeaders.CONTENT_DISPOSITION,
String.format("attachment;filename=\"%s.%s\"", name, format))
+ .build();
}
@DELETE
@Path("/storage/{name}.json")
@Produces(MediaType.APPLICATION_JSON)
- public JsonResult deletePlugin(@PathParam("name") String name) {
+ public Response deletePlugin(@PathParam("name") String name) {
try {
storage.remove(name);
- return message("Success");
+ return Response.ok().entity(message("Success")).build();
} catch (PluginException e) {
- return message(e.getMessage());
+ return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
+ .entity(message("Error while deleting plugin: %s", e.getMessage()))
+ .build();
}
}
@@ -217,13 +231,15 @@ public class StorageResources {
@Path("/storage/{name}.json")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
- public JsonResult createOrUpdatePluginJSON(PluginConfigWrapper plugin) {
+ public Response createOrUpdatePluginJSON(PluginConfigWrapper plugin) {
try {
plugin.createOrUpdateInStorage(storage);
- return message("Success");
+ return Response.ok().entity(message("Success")).build();
} catch (PluginException e) {
logger.error("Unable to create/ update plugin: " + plugin.getName(), e);
- return message("Error while saving plugin: %s", e.getMessage());
+ return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
+ .entity(message("Error while saving plugin: %s ", e.getMessage()))
+ .build();
}
}
@@ -236,20 +252,27 @@ public class StorageResources {
@Path("/storage/create_update")
@Consumes(MediaType.APPLICATION_FORM_URLENCODED)
@Produces(MediaType.APPLICATION_JSON)
- public JsonResult createOrUpdatePlugin(@FormParam("name") String name,
@FormParam("config") String storagePluginConfig) {
+ public Response createOrUpdatePlugin(@FormParam("name") String name,
@FormParam("config") String storagePluginConfig) {
name = name.trim();
if (name.isEmpty()) {
- return message("Error (a storage name cannot be empty)");
+ return Response.status(Response.Status.BAD_REQUEST)
+ .entity(message("A storage config name may not be empty"))
+ .build();
}
+
try {
storage.putJson(name, storagePluginConfig);
- return message("Success");
+ return Response.ok().entity(message("Success")).build();
} catch (PluginEncodingException e) {
logger.warn("Error in JSON mapping: {}", storagePluginConfig, e);
- return message("Invalid JSON: " + e.getMessage());
+ return Response.status(Response.Status.BAD_REQUEST)
+ .entity(message("Invalid JSON: %s", e.getMessage()))
+ .build();
} catch (PluginException e) {
logger.error("Error while saving plugin", e);
- return message(e.getMessage());
+ return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
+ .entity(message("Error while saving plugin: ", e.getMessage()))
+ .build();
}
}
@@ -310,7 +333,7 @@ public class StorageResources {
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
@Deprecated
- public JsonResult createOrUpdatePlugin(PluginConfigWrapper plugin) {
+ public Response createOrUpdatePlugin(PluginConfigWrapper plugin) {
return createOrUpdatePluginJSON(plugin);
}
@@ -321,7 +344,7 @@ public class StorageResources {
@Path("/storage/{name}/delete")
@Produces(MediaType.APPLICATION_JSON)
@Deprecated
- public JsonResult deletePluginViaGet(@PathParam("name") String name) {
+ public Response deletePluginViaGet(@PathParam("name") String name) {
return deletePlugin(name);
}
@@ -341,7 +364,7 @@ public class StorageResources {
/**
* Model class for Storage Plugin page.
- * It contains a storage plugin as well as the CSRK token for the page.
+ * It contains a storage plugin as well as the CSRF token for the page.
*/
public static class StoragePluginModel {
private final PluginConfigWrapper plugin;
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 288df87..127b2bb 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
@@ -36,6 +36,7 @@ import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.core.Context;
import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
import javax.ws.rs.core.SecurityContext;
import javax.ws.rs.core.UriInfo;
import javax.xml.bind.annotation.XmlRootElement;
@@ -257,7 +258,7 @@ public class ProfileResources {
@GET
@Path("/profiles.json")
@Produces(MediaType.APPLICATION_JSON)
- public QProfiles getProfilesJSON(@Context UriInfo uriInfo) {
+ public Response getProfilesJSON(@Context UriInfo uriInfo) {
try {
final QueryProfileStoreContext profileStoreContext =
work.getContext().getProfileStoreContext();
final PersistentStore<QueryProfile> completed =
profileStoreContext.getCompletedProfileStore();
@@ -319,7 +320,14 @@ public class ProfileResources {
Collections.sort(finishedQueries, Collections.reverseOrder());
- return new QProfiles(runningQueries, finishedQueries, errors);
+
+ QProfiles qProf = new QProfiles(runningQueries, finishedQueries, errors);
+
+ return errors.size() == 0
+ ? Response.ok().entity(qProf).build()
+ : Response.status(Response.Status.INTERNAL_SERVER_ERROR)
+ .entity(qProf)
+ .build();
} catch (Exception e) {
throw UserException.resourceError(e)
.message("Failed to get profiles from persistent or ephemeral store.")
@@ -331,7 +339,7 @@ public class ProfileResources {
@Path("/profiles")
@Produces(MediaType.TEXT_HTML)
public Viewable getProfiles(@Context UriInfo uriInfo) {
- QProfiles profiles = getProfilesJSON(uriInfo);
+ QProfiles profiles = (QProfiles) getProfilesJSON(uriInfo).getEntity();
return ViewableWithPermissions.create(authEnabled.get(),
"/rest/profile/list.ftl", sc, profiles);
}
@@ -383,19 +391,23 @@ public class ProfileResources {
@GET
@Path("/profiles/{queryid}.json")
@Produces(MediaType.APPLICATION_JSON)
- public String getProfileJSON(@PathParam("queryid") String queryId) {
+ public Response getProfileJSON(@PathParam("queryid") String queryId) {
try {
String profileData = PROFILE_CACHE.getIfPresent(queryId);
if (profileData == null) {
- return new String(work.getContext().getProfileStoreContext()
+ profileData = new String(work.getContext().getProfileStoreContext()
.getProfileStoreConfig().getSerializer().serialize(getQueryProfile(queryId)));
} else {
PROFILE_CACHE.invalidate(queryId);
- return profileData;
}
+
+ return Response.ok().entity(profileData).build();
+
} catch (Exception e) {
logger.debug("Failed to serialize profile for: " + queryId);
- return ("{ 'message' : 'error (unable to serialize profile)' }");
+ return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
+ .entity("{ 'message' : 'error (unable to serialize profile)' }")
+ .build();
}
}
diff --git a/exec/java-exec/src/main/resources/rest/static/js/serverMessage.js
b/exec/java-exec/src/main/resources/rest/static/js/serverMessage.js
index d426f25..dab35d8 100644
--- a/exec/java-exec/src/main/resources/rest/static/js/serverMessage.js
+++ b/exec/java-exec/src/main/resources/rest/static/js/serverMessage.js
@@ -22,6 +22,8 @@ function serverMessage(data) {
setTimeout(function() { window.location.href = "/storage"; }, 800);
return true;
} else {
+ const errorMessage = data.errorMessage || data.result;
+
messageEl.addClass("d-none");
// Wait a fraction of a second before showing the message again. This
// makes it clear if a second attempt gives the same error as
@@ -30,7 +32,7 @@ function serverMessage(data) {
messageEl.removeClass("d-none")
.removeClass("alert-info")
.addClass("alert-danger")
- .text("Please retry: " + data.result).alert();
+ .text("Please retry: " + errorMessage).alert();
}, 200);
return false;
}
diff --git a/exec/java-exec/src/main/resources/rest/storage/list.ftl
b/exec/java-exec/src/main/resources/rest/storage/list.ftl
index 5d4b6ae..370eaa1 100644
--- a/exec/java-exec/src/main/resources/rest/storage/list.ftl
+++ b/exec/java-exec/src/main/resources/rest/storage/list.ftl
@@ -131,7 +131,8 @@
</div>
<div class="radio">
<label>
- <input type="radio" name="format" id="hocon" value="conf">
+ <!-- Exporting to HOCON is not currently supported, see
StorageResources.java. -->
+ <input type="radio" name="format" id="hocon" value="conf"
disabled>
HOCON
</label>
</div>
@@ -214,8 +215,13 @@
if (data.result === "Success") {
location.reload();
} else {
- populateAndShowAlert('pluginEnablingFailure', {'_pluginName_':
name,'_errorMessage_': data.result});
+ populateAndShowAlert('pluginEnablingFailure', {'_pluginName_':
name,'_errorMessage_': data.result});
}
+ }).fail(function(response) {
+ populateAndShowAlert(
+ 'pluginEnablingFailure',
+ {'_pluginName_': name,'_errorMessage_':
response.responseJSON.result}
+ );
});
}
}
@@ -227,7 +233,8 @@
function doCreate() {
$("#createForm").ajaxForm({
dataType: 'json',
- success: serverMessage
+ success: serverMessage,
+ error: serverMessage
});
}
@@ -254,7 +261,6 @@
// Modal windows management
let exportInstance; // global variable
$('#pluginsModal').on('show.bs.modal', function(event) {
- console.log("alarm");
const button = $(event.relatedTarget); // Button that triggered the modal
const modal = $(this);
exportInstance = button.attr("name");
@@ -297,4 +303,4 @@
<#include "*/alertModals.ftl">
</#macro>
-<@page_html/>
\ No newline at end of file
+<@page_html/>
diff --git a/exec/java-exec/src/main/resources/rest/storage/update.ftl
b/exec/java-exec/src/main/resources/rest/storage/update.ftl
index 224eb52..285771f 100644
--- a/exec/java-exec/src/main/resources/rest/storage/update.ftl
+++ b/exec/java-exec/src/main/resources/rest/storage/update.ftl
@@ -125,6 +125,10 @@
if (serverMessage(data)) {
setTimeout(function() { location.reload(); }, 800);
}
+ }).fail(function(response) {
+ if (serverMessage(response.responseJSON)) {
+ setTimeout(function() { location.reload(); }, 800);
+ }
});
}
});
@@ -132,13 +136,16 @@
function doUpdate() {
$("#updateForm").ajaxForm({
dataType: 'json',
- success: serverMessage
+ success: serverMessage,
+ error: serverMessage
});
}
function deleteFunction() {
showConfirmationDialog('"${model.getPlugin().getName()}"' + ' plugin
will be deleted. Proceed?', function() {
- $.get("/storage/" +
encodeURIComponent("${model.getPlugin().getName()}") + "/delete",
serverMessage);
+ $.get("/storage/" +
encodeURIComponent("${model.getPlugin().getName()}") + "/delete",
serverMessage).fail(function() {
+ serverMessage({ errorMessage: "Error
while trying to delete." })
+ });
});
}
@@ -164,4 +171,4 @@
</script>
</#macro>
-<@page_html/>
\ No newline at end of file
+<@page_html/>
