cgivre commented on code in PR #2650: URL: https://github.com/apache/drill/pull/2650#discussion_r973137352
########## contrib/storage-druid/src/main/java/org/apache/drill/exec/store/druid/rest/DruidQueryClient.java: ########## @@ -48,20 +48,21 @@ public DruidQueryClient(String brokerURI, RestClient restClient) { public DruidScanResponse executeQuery(String query) throws Exception { logger.debug("Executing Query - {}", query); - HttpResponse response = restClient.post(queryUrl, query); - if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) { - throw UserException - .dataReadError() - .message("Error executing druid query. HTTP request failed") - .addContext("Response code", response.getStatusLine().getStatusCode()) - .addContext("Response message", response.getStatusLine().getReasonPhrase()) - .build(logger); - } + try (Response response = restClient.post(queryUrl, query)) { + if (!response.isSuccessful()) { + throw UserException Review Comment: Would it be possible to get the `errorContext` from the `DruidBatchReader` so that we get good error messages? ########## contrib/storage-druid/src/main/java/org/apache/drill/exec/store/druid/rest/DruidQueryClient.java: ########## @@ -48,20 +48,21 @@ public DruidQueryClient(String brokerURI, RestClient restClient) { public DruidScanResponse executeQuery(String query) throws Exception { logger.debug("Executing Query - {}", query); - HttpResponse response = restClient.post(queryUrl, query); - if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) { - throw UserException - .dataReadError() - .message("Error executing druid query. HTTP request failed") - .addContext("Response code", response.getStatusLine().getStatusCode()) - .addContext("Response message", response.getStatusLine().getReasonPhrase()) - .build(logger); - } + try (Response response = restClient.post(queryUrl, query)) { + if (!response.isSuccessful()) { + throw UserException Review Comment: I see that these objects are created in the `DruidPlguin` but maybe we could add a method `addErrorContext` or something like that and call it in the constructor of the Batch Reader. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org