From 27e72499a3a80c0b2927d532d2d4959d8be4eea6 Mon Sep 17 00:00:00 2001 From: Charles Givre Date: Mon, 16 Mar 2020 12:35:55 -0400 Subject: [PATCH] Addressed Review Comments --- common/pom.xml | 2 - contrib/storage-http/README.md | 54 +++++++++---------- .../drill/exec/store/http/HttpAPIConfig.java | 47 ++++++++++------ .../drill/exec/store/http/HttpGroupScan.java | 45 +++++++++------- .../exec/store/http/HttpRecordReader.java | 4 +- .../exec/store/http/HttpScanBatchCreator.java | 9 ++-- .../drill/exec/store/http/HttpScanSpec.java | 12 ++--- .../exec/store/http/HttpSchemaFactory.java | 16 +++--- .../exec/store/http/HttpStoragePlugin.java | 8 +-- .../store/http/HttpStoragePluginConfig.java | 22 +++++--- .../drill/exec/store/http/HttpSubScan.java | 14 +++-- .../exec/store/http/util/SimpleHttp.java | 34 ++++-------- .../resources/bootstrap-storage-plugins.json | 2 +- .../src/main/resources/drill-module.conf | 2 +- .../drill/exec/store/http/TestHttpPlugin.java | 15 ++++-- .../src/test/resources/data/response.json | 14 +++++ ...{logback-test.xml => logback-test.xml.bak} | 0 .../drill/exec/util/DirectoryUtils.java | 47 ++++++++++++++++ 18 files changed, 205 insertions(+), 142 deletions(-) create mode 100644 contrib/storage-http/src/test/resources/data/response.json rename contrib/storage-http/src/test/resources/{logback-test.xml => logback-test.xml.bak} (100%) create mode 100644 exec/java-exec/src/main/java/org/apache/drill/exec/util/DirectoryUtils.java diff --git a/common/pom.xml b/common/pom.xml index 889275e152b..8198be62537 100644 --- a/common/pom.xml +++ b/common/pom.xml @@ -37,14 +37,12 @@ drill-protocol ${project.version} - junit junit ${junit.version} - com.typesafe config diff --git a/contrib/storage-http/README.md b/contrib/storage-http/README.md index b6cd9a5f058..412f14e0285 100644 --- a/contrib/storage-http/README.md +++ b/contrib/storage-http/README.md @@ -33,8 +33,8 @@ The `connection` can accept the following options: . The format is: ```json headers: { - "key1":, "Value1", - "key2", "Value2" + "key1": "Value1", + "key2": "Value2" } ``` * `authType`: If your API requires authentication, specify the authentication type. At the time of implementation, the plugin only supports basic authentication, however, the @@ -74,45 +74,44 @@ FROM .. The API sunrise-sunset.org returns data in the following format: ```json - { - "results": - { - "sunrise":"7:27:02 AM", - "sunset":"5:05:55 PM", - "solar_noon":"12:16:28 PM", - "day_length":"9:38:53", - "civil_twilight_begin":"6:58:14 AM", - "civil_twilight_end":"5:34:43 PM", - "nautical_twilight_begin":"6:25:47 AM", - "nautical_twilight_end":"6:07:10 PM", - "astronomical_twilight_begin":"5:54:14 AM", - "astronomical_twilight_end":"6:38:43 PM" - }, - "status":"OK" - } - } + "results": + { + "sunrise":"7:27:02 AM", + "sunset":"5:05:55 PM", + "solar_noon":"12:16:28 PM", + "day_length":"9:38:53", + "civil_twilight_begin":"6:58:14 AM", + "civil_twilight_end":"5:34:43 PM", + "nautical_twilight_begin":"6:25:47 AM", + "nautical_twilight_end":"6:07:10 PM", + "astronomical_twilight_begin":"5:54:14 AM", + "astronomical_twilight_end":"6:38:43 PM" + }, + "status":"OK" +} ``` To query this API, set the configuration as follows: ```json -{ + { "type": "http", "cacheResults": false, "enabled": true, - "timeout" 5, + "timeout": 5, "connections": { "sunrise": { "url": "https://api.sunrise-sunset.org/", - "method": "get", + "method": "GET", "headers": null, "authType": "none", "userName": null, "password": null, "postBody": null } - }, + } } + ``` Then, to execute a query: ```sql @@ -138,11 +137,11 @@ To connect Drill to JIRA Cloud, use the following configuration: { "type": "http", "cacheResults": false, - "timeout" 5, + "timeout": 5, "connections": { "sunrise": { "url": "https://api.sunrise-sunset.org/", - "method": "get", + "method": "GET", "headers": null, "authType": "none", "userName": null, @@ -151,7 +150,7 @@ To connect Drill to JIRA Cloud, use the following configuration: }, "jira": { "url": "https://.atlassian.net/rest/api/3/", - "method": "get", + "method": "GET", "headers": { "Accept": "application/json" }, @@ -212,7 +211,8 @@ ORDER BY issue_count DESC 3. This plugin does not implement filter pushdowns. Filter pushdown has the potential to improve performance. 4. This plugin only reads JSON responses. Future functionality may include the ability to parse XML, CSV or other common rest responses. - + + 5. At this time `POST` bodies can only be in the format of KV pairs. Some APIs accept JSON based `POST` bodies and this is not currently supported. diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpAPIConfig.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpAPIConfig.java index 48102f9df00..bb192ffad5e 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpAPIConfig.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpAPIConfig.java @@ -19,13 +19,17 @@ package org.apache.drill.exec.store.http; import com.fasterxml.jackson.annotation.JsonProperty; -import org.apache.drill.shaded.guava.com.google.common.base.MoreObjects; +import org.apache.drill.common.PlanStringBuilder; +import org.apache.parquet.Strings; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.Arrays; import java.util.Map; import java.util.Objects; public class HttpAPIConfig { + private static final Logger logger = LoggerFactory.getLogger(HttpAPIConfig.class); private final String url; @@ -49,12 +53,24 @@ public HttpAPIConfig(@JsonProperty("url") String url, @JsonProperty("password") String password, @JsonProperty("postBody") String postBody) { + if (method == null) { + method = "GET"; + } + // Get the request method. Only accept GET and POST requests. Anything else will default to GET. - if (method.toLowerCase().equals("get") || method.toLowerCase().equals("post")) { - this.method = method.toLowerCase(); - } else { - this.method = "get"; + switch (method) { + case "get": + case "GET": + case "post": + case "POST": + this.method = method.toUpperCase(); + break; + default: + // Case for null or other HTTP Request types + logger.warn("HTTP/REST Plugin only supports GET and POST requests. Current config for {} uses method {}.", url, method); + this.method = "GET"; } + this.headers = headers; // Put a trailing slash on the URL if it is missing @@ -66,13 +82,10 @@ public HttpAPIConfig(@JsonProperty("url") String url, // Get the authentication method. Future functionality will include OAUTH2 authentication but for now // Accept either basic or none. The default is none. - this.authType = (authType == null || authType.isEmpty()) ? "none" : authType; + this.authType = Strings.isNullOrEmpty(authType) ? "none" : authType; this.userName = userName; this.password = password; this.postBody = postBody; - - // Validate the authentication type - } @JsonProperty("url") @@ -104,14 +117,14 @@ public int hashCode() { @Override public String toString() { - return MoreObjects.toStringHelper(this) - .add("url", url) - .add("method", method) - .add("headers", headers) - .add("authType", authType) - .add("username", userName) - .add("password", password) - .add("postBody", postBody) + return new PlanStringBuilder(this) + .field("url", url) + .field("method", method) + .field("headers", headers) + .field("authType", authType) + .field("username", userName) + .field("password", password) + .field("postBody", postBody) .toString(); } diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpGroupScan.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpGroupScan.java index 27891bf9b47..4f3ff428a51 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpGroupScan.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpGroupScan.java @@ -17,7 +17,6 @@ */ package org.apache.drill.exec.store.http; -import java.util.Arrays; import java.util.List; import java.util.Objects; @@ -25,6 +24,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; +import org.apache.drill.common.PlanStringBuilder; import org.apache.drill.common.expression.SchemaPath; import org.apache.drill.exec.physical.base.AbstractGroupScan; @@ -33,17 +33,15 @@ import org.apache.drill.exec.physical.base.ScanStats; import org.apache.drill.exec.physical.base.ScanStats.GroupScanProperty; import org.apache.drill.exec.physical.base.SubScan; +import org.apache.drill.exec.planner.cost.DrillCostBase; import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint; -import org.apache.drill.shaded.guava.com.google.common.base.MoreObjects; import org.apache.drill.shaded.guava.com.google.common.base.Preconditions; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; + @JsonTypeName("http-scan") public class HttpGroupScan extends AbstractGroupScan { - private static final Logger logger = LoggerFactory.getLogger(HttpGroupScan.class); - private List columns; + private final List columns; private final HttpScanSpec httpScanSpec; private final HttpStoragePluginConfig config; @@ -65,6 +63,13 @@ public HttpGroupScan(HttpGroupScan that) { columns = that.getColumns(); } + public HttpGroupScan(HttpGroupScan that, List columns) { + super("no-user"); + this.columns = columns; + this.config = that.config; + this.httpScanSpec = that.httpScanSpec; + } + @JsonCreator public HttpGroupScan( @JsonProperty("config") HttpStoragePluginConfig config, @@ -88,7 +93,8 @@ public HttpGroupScan( @Override public void applyAssignments(List endpoints) { - logger.debug("HttpGroupScan applyAssignments"); + // No filter pushdowns yet, so this method does nothing + return; } @Override @@ -104,15 +110,12 @@ public boolean canPushdownProjects(List columns) { @Override public SubScan getSpecificScan(int minorFragmentId) { - logger.debug("HttpGroupScan getSpecificScan"); return new HttpSubScan(config, httpScanSpec, columns); } @Override public GroupScan clone(List columns) { - logger.debug("HttpGroupScan clone {}", columns); - HttpGroupScan newScan = new HttpGroupScan(this); - newScan.columns = columns; + HttpGroupScan newScan = new HttpGroupScan(this, columns); return newScan; } @@ -129,26 +132,28 @@ public PhysicalOperator getNewWithChildren(List children) { @Override public ScanStats getScanStats() { - int colCount = columns.size(); int estRowCount = 1; - int estDataSize = estRowCount * 200 * colCount; - int estCpuCost = 1; + + int rowWidth = 200; + if (columns.size() == 0) { rowWidth = 100; } + + int estDataSize = estRowCount * 200 * rowWidth; + int estCpuCost = DrillCostBase.PROJECT_CPU_COST; return new ScanStats(GroupScanProperty.NO_EXACT_ROW_COUNT,estRowCount, estCpuCost, estDataSize); } @Override public String toString() { - return MoreObjects.toStringHelper(this) - .add("httpScanSpec", httpScanSpec) - .add("columns", columns) - .add("httpStoragePluginConfig", config) + return new PlanStringBuilder(this) + .field("httpScanSpec", httpScanSpec) + .field("columns", columns) + .field("httpStoragePluginConfig", config) .toString(); } @Override public int hashCode() { - return Arrays.hashCode( - new Object[]{httpScanSpec, columns, config}); + return Objects.hash(httpScanSpec, columns, config); } @Override diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpRecordReader.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpRecordReader.java index 76dac5b5e80..7082de8f589 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpRecordReader.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpRecordReader.java @@ -18,6 +18,7 @@ package org.apache.drill.exec.store.http; +import org.apache.drill.common.exceptions.UserException; import org.apache.drill.common.expression.SchemaPath; import org.apache.drill.exec.ops.FragmentContext; import org.apache.drill.exec.store.easy.json.JSONRecordReader; @@ -45,8 +46,9 @@ public HttpRecordReader(FragmentContext context, List projectedColum /** * Executes the HTTP request and returns an InputStream to the retrieved data * @return InputStream the InputStream containing the data + * @throws UserException Throws a UserException if unable to return an open InputStream. */ - private InputStream getInputStream() { + private InputStream getInputStream() throws UserException { String url = subScan.getFullURL(); return http.getInputStream(url); } diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpScanBatchCreator.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpScanBatchCreator.java index 1fb67414f83..9d4ba1fc1b1 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpScanBatchCreator.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpScanBatchCreator.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.http; +import java.util.ArrayList; import java.util.List; import org.apache.drill.common.exceptions.ExecutionSetupException; @@ -28,7 +29,6 @@ import org.apache.drill.exec.record.RecordBatch; import org.apache.drill.exec.store.RecordReader; import org.apache.drill.shaded.guava.com.google.common.base.Preconditions; -import org.apache.drill.shaded.guava.com.google.common.collect.Lists; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,12 +41,9 @@ public ScanBatch getBatch(ExecutorFragmentContext context, HttpSubScan subScan, Preconditions.checkArgument(children == null || children.isEmpty()); HttpStoragePluginConfig config = subScan.config(); - List readers = Lists.newArrayList(); - List columns; + List readers = new ArrayList<>(); - if ((columns = subScan.columns()) == null) { - columns = GroupScan.ALL_COLUMNS; - } + List columns = subScan.columns() == null ? GroupScan.ALL_COLUMNS : subScan.columns(); readers.add(new HttpRecordReader(context, columns, config, subScan)); return new ScanBatch(subScan, context, readers); diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpScanSpec.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpScanSpec.java index c263a503ee3..19921bc8a42 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpScanSpec.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpScanSpec.java @@ -21,7 +21,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; -import org.apache.drill.shaded.guava.com.google.common.base.MoreObjects; +import org.apache.drill.common.PlanStringBuilder; @JsonTypeName("http-scan-spec") public class HttpScanSpec { @@ -62,11 +62,11 @@ public String getURL() { @Override public String toString() { - return MoreObjects.toStringHelper(this) - .add("schemaName", schemaName) - .add("database", database) - .add("tableName", tableName) - .add("config", config) + return new PlanStringBuilder(this) + .field("schemaName", schemaName) + .field("database", database) + .field("tableName", tableName) + .field("config", config) .toString(); } } diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpSchemaFactory.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpSchemaFactory.java index ac0e9e76e19..c111d3df547 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpSchemaFactory.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpSchemaFactory.java @@ -23,7 +23,6 @@ import java.util.Set; import org.apache.calcite.schema.SchemaPlus; -import org.apache.drill.common.exceptions.DrillRuntimeException; import org.apache.drill.common.exceptions.UserException; import org.apache.drill.exec.store.AbstractSchema; import org.apache.drill.exec.store.AbstractSchemaFactory; @@ -77,16 +76,13 @@ public Set getSubSchemaNames() { @Override public AbstractSchema getSubSchema(String name) { - try { - if (!plugin.getConfig().connections().containsKey(name)) { - throw UserException - .connectionError() - .message("API '{}' does not exist in HTTP Storage plugin '{}'", name, getName()) - .build(logger); - } + if (plugin.getConfig().connections().containsKey(name)) { return getSubSchemaKnownExists(name); - } catch (Exception e) { - throw new DrillRuntimeException(e); + } else { + throw UserException + .connectionError() + .message("API '{}' does not exist in HTTP Storage plugin '{}'", name, getName()) + .build(logger); } } diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpStoragePlugin.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpStoragePlugin.java index 302e460cf16..0826be62389 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpStoragePlugin.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpStoragePlugin.java @@ -24,7 +24,6 @@ import org.apache.drill.exec.server.DrillbitContext; import org.apache.drill.exec.store.AbstractStoragePlugin; import org.apache.drill.exec.store.SchemaConfig; -import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.core.type.TypeReference; import java.io.IOException; @@ -53,11 +52,6 @@ public HttpStoragePluginConfig getConfig() { return engineConfig; } - @Override - public DrillbitContext getContext() { - return context; - } - @Override public boolean supportsRead() { return true; @@ -65,7 +59,7 @@ public boolean supportsRead() { @Override public AbstractGroupScan getPhysicalScan(String userName, JSONOptions selection) throws IOException { - HttpScanSpec scanSpec = selection.getListWith(new ObjectMapper(), new TypeReference() {}); + HttpScanSpec scanSpec = selection.getListWith(context.getLpPersistence().getMapper(), new TypeReference() {}); return new HttpGroupScan(engineConfig, scanSpec, null); } } diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpStoragePluginConfig.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpStoragePluginConfig.java index 058253fbd42..694f55862f3 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpStoragePluginConfig.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpStoragePluginConfig.java @@ -17,11 +17,10 @@ */ package org.apache.drill.exec.store.http; +import org.apache.drill.common.PlanStringBuilder; import org.apache.drill.common.map.CaseInsensitiveMap; import org.apache.drill.common.logical.StoragePluginConfigBase; import org.apache.drill.shaded.guava.com.google.common.base.Objects; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; @@ -34,7 +33,6 @@ @JsonTypeName(HttpStoragePluginConfig.NAME) public class HttpStoragePluginConfig extends StoragePluginConfigBase { - private static final Logger logger = LoggerFactory.getLogger(HttpStoragePluginConfig.class); public static final String NAME = "http"; @@ -48,15 +46,14 @@ public class HttpStoragePluginConfig extends StoragePluginConfigBase { public HttpStoragePluginConfig(@JsonProperty("cacheResults") boolean cacheResults, @JsonProperty("connections") Map connections, @JsonProperty("timeout") int timeout) { - logger.debug("Initialize HttpStoragePluginConfig {}", connections); this.cacheResults = cacheResults; - if (connections != null) { + if (connections == null) { + this.connections = new HashMap<>(); + } else { Map caseInsensitiveAPIs = CaseInsensitiveMap.newHashMap(); Optional.of(connections).ifPresent(caseInsensitiveAPIs::putAll); this.connections = caseInsensitiveAPIs; - } else { - this.connections = new HashMap<>(); } this.timeout = timeout; @@ -74,9 +71,18 @@ public boolean equals(Object that) { this.connections.equals(thatConfig.connections); } + @Override + public String toString() { + return new PlanStringBuilder(this) + .field("connections", connections) + .field("cacheResults", cacheResults) + .field("timeout", timeout) + .toString(); + } + @Override public int hashCode() { - return Objects.hashCode(cacheResults, connections); + return Objects.hashCode(cacheResults, connections, timeout); } @JsonProperty("cacheResults") diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpSubScan.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpSubScan.java index 9538b531c3f..700ad70af52 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpSubScan.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpSubScan.java @@ -17,7 +17,6 @@ */ package org.apache.drill.exec.store.http; -import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.Objects; @@ -26,13 +25,13 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; +import org.apache.drill.common.PlanStringBuilder; import org.apache.drill.common.expression.SchemaPath; import org.apache.drill.exec.physical.base.AbstractBase; import org.apache.drill.exec.physical.base.PhysicalOperator; import org.apache.drill.exec.physical.base.PhysicalVisitor; import org.apache.drill.exec.physical.base.SubScan; import org.apache.drill.exec.proto.UserBitShared.CoreOperatorType; -import org.apache.drill.shaded.guava.com.google.common.base.MoreObjects; import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableSet; @JsonTypeName("http-sub-scan") @@ -103,17 +102,16 @@ public Iterator iterator() { @Override public String toString() { - return MoreObjects.toStringHelper(this) - .add("tableSpec", tableSpec) - .add("columns", columns) - .add("config", config) + return new PlanStringBuilder(this) + .field("tableSpec", tableSpec) + .field("columns", columns) + .field("config", config) .toString(); } @Override public int hashCode() { - return Arrays.hashCode( - new Object[]{tableSpec, columns, config}); + return Objects.hash(tableSpec,columns,config); } @Override diff --git a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java index 7e6515e0baf..7247b50bd3b 100644 --- a/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java +++ b/contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java @@ -26,8 +26,8 @@ import okhttp3.Request; import okhttp3.Response; +import org.apache.drill.exec.util.DirectoryUtils; import org.apache.drill.common.exceptions.UserException; -import org.apache.drill.exec.ExecConstants; import org.apache.drill.exec.ops.FragmentContext; import org.apache.drill.exec.store.http.HttpAPIConfig; import org.apache.drill.exec.store.http.HttpStoragePluginConfig; @@ -69,30 +69,24 @@ public SimpleHttp(HttpStoragePluginConfig config, FragmentContext context, Strin public InputStream getInputStream(String urlStr) { Request.Builder requestBuilder; + requestBuilder = new Request.Builder().url(urlStr); + // The configuration does not allow for any other request types other than POST and GET. - if (apiConfig.method().equals("get")) { - // Handle GET requests - requestBuilder = new Request.Builder().url(urlStr); - } else { + if (apiConfig.method().equals("POST")) { // Handle POST requests FormBody.Builder formBodyBuilder = buildPostBody(); - requestBuilder = new Request.Builder() - .url(urlStr) - .post(formBodyBuilder.build()); + requestBuilder.post(formBodyBuilder.build()); } // Add headers to request if (apiConfig.headers() != null) { for (Map.Entry entry : apiConfig.headers().entrySet()) { - String key = entry.getKey(); - String value = entry.getValue(); - requestBuilder.addHeader(key, value); + requestBuilder.addHeader(entry.getKey(), entry.getValue()); } } // Build the request object Request request = requestBuilder.build(); - logger.debug("Headers: {}", request.headers()); try { // Execute the request @@ -166,18 +160,14 @@ private void setupCache(Builder builder) { String drillTempDir; try { - if (context.getOptions().getOption(ExecConstants.DRILL_TMP_DIR) != null) { - drillTempDir = context.getOptions().getOption(ExecConstants.DRILL_TMP_DIR).string_val; - } else { - drillTempDir = System.getenv("DRILL_TMP_DIR"); - } + drillTempDir = DirectoryUtils.getDrillTempDirectory(context); + File cacheDirectory = new File(drillTempDir); if (cacheDirectory == null) { logger.warn("HTTP Storage plugin caching requires the DRILL_TMP_DIR to be configured. Please either set DRILL_TMP_DIR or disable HTTP caching."); } else { Cache cache = new Cache(cacheDirectory, cacheSize); logger.debug("Caching HTTP Query Results at: {}", drillTempDir); - builder.cache(cache); } } catch (Exception e) { @@ -192,10 +182,10 @@ private void setupCache(Builder builder) { * * and creates the appropriate headers. * - * @return FormBodu.Builder The populated formbody builder + * @return FormBody.Builder The populated formbody builder */ private FormBody.Builder buildPostBody() { - final Pattern postBpdyPattern = Pattern.compile("^.+=.+$"); + final Pattern postBodyPattern = Pattern.compile("^.+=.+$"); FormBody.Builder formBodyBuilder = new FormBody.Builder(); String[] lines = apiConfig.postBody().split("\\r?\\n"); @@ -203,7 +193,7 @@ private FormBody.Builder buildPostBody() { // If the string is in the format key=value split it, // Otherwise ignore - if (postBpdyPattern.matcher(line).find()) { + if (postBodyPattern.matcher(line).find()) { //Split into key/value String[] parts = line.split("="); formBodyBuilder.add(parts[0], parts[1]); @@ -220,13 +210,11 @@ public static class BasicAuthInterceptor implements Interceptor { public BasicAuthInterceptor(String user, String password) { credentials = Credentials.basic(user, password); - logger.debug("Intercepting request adding creds: {}", credentials); } @NotNull @Override public Response intercept(Chain chain) throws IOException { - logger.debug("Adding headers post intercept{}", credentials); // Get the existing request Request request = chain.request(); diff --git a/contrib/storage-http/src/main/resources/bootstrap-storage-plugins.json b/contrib/storage-http/src/main/resources/bootstrap-storage-plugins.json index 25bcf234b01..9afcaeeab17 100644 --- a/contrib/storage-http/src/main/resources/bootstrap-storage-plugins.json +++ b/contrib/storage-http/src/main/resources/bootstrap-storage-plugins.json @@ -6,4 +6,4 @@ "enabled": false } } -} \ No newline at end of file +} diff --git a/contrib/storage-http/src/main/resources/drill-module.conf b/contrib/storage-http/src/main/resources/drill-module.conf index b4a33e6b42d..c0f2a9327cb 100644 --- a/contrib/storage-http/src/main/resources/drill-module.conf +++ b/contrib/storage-http/src/main/resources/drill-module.conf @@ -24,4 +24,4 @@ drill: { classpath.scanning: { packages += "org.apache.drill.exec.store.http" } -} \ No newline at end of file +} diff --git a/contrib/storage-http/src/test/java/org/apache/drill/exec/store/http/TestHttpPlugin.java b/contrib/storage-http/src/test/java/org/apache/drill/exec/store/http/TestHttpPlugin.java index 7dafdefbb86..afb4adf07f0 100644 --- a/contrib/storage-http/src/test/java/org/apache/drill/exec/store/http/TestHttpPlugin.java +++ b/contrib/storage-http/src/test/java/org/apache/drill/exec/store/http/TestHttpPlugin.java @@ -17,6 +17,7 @@ */ package org.apache.drill.exec.store.http; + import static org.apache.drill.test.rowSet.RowSetUtilities.mapValue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -29,11 +30,14 @@ import okio.Buffer; import okio.Okio; import org.apache.drill.common.types.TypeProtos; +import org.apache.drill.common.util.DrillFileUtils; import org.apache.drill.exec.physical.rowSet.RowSet; import org.apache.drill.exec.physical.rowSet.RowSetBuilder; import org.apache.drill.exec.record.metadata.SchemaBuilder; import org.apache.drill.exec.record.metadata.TupleMetadata; import org.apache.drill.exec.store.StoragePluginRegistry; +import org.apache.drill.shaded.guava.com.google.common.io.Files; +import org.apache.drill.shaded.guava.com.google.common.base.Charsets; import org.apache.drill.test.ClusterFixture; import org.apache.drill.test.ClusterTest; import org.apache.drill.test.rowSet.RowSetComparison; @@ -61,13 +65,14 @@ public class TestHttpPlugin extends ClusterTest { private static final Logger logger = LoggerFactory.getLogger(TestHttpPlugin.class); - private final String TEST_JSON_RESPONSE = "{\"results\":{\"sunrise\":\"6:13:58 AM\",\"sunset\":\"5:59:55 PM\",\"solar_noon\":\"12:06:56 PM\",\"day_length\":\"11:45:57\"," + - "\"civil_twilight_begin\":\"5:48:14 AM\",\"civil_twilight_end\":\"6:25:38 PM\",\"nautical_twilight_begin\":\"5:18:16 AM\",\"nautical_twilight_end\":\"6:55:36 PM\",\"astronomical_twilight_begin\":\"4:48:07 AM\",\"astronomical_twilight_end\":\"7:25:45 PM\"},\"status\":\"OK\"}"; + private static String TEST_JSON_RESPONSE; @BeforeClass public static void setup() throws Exception { startCluster(ClusterFixture.builder(dirTestWatcher)); + TEST_JSON_RESPONSE = Files.asCharSource(DrillFileUtils.getResourceAsFile("/data/response.json"), Charsets.UTF_8).read(); + dirTestWatcher.copyResourceToRoot(Paths.get("data/")); StoragePluginRegistry pluginRegistry = cluster.drillbit().getContext().getStorage(); @@ -76,14 +81,14 @@ public static void setup() throws Exception { headers.put("header1", "value1"); headers.put("header2", "value2"); - HttpAPIConfig mockConfig = new HttpAPIConfig("http://localhost:8091/", "get", headers, "basic", "user", "pass",null); + HttpAPIConfig mockConfig = new HttpAPIConfig("http://localhost:8091/", "GET", headers, "basic", "user", "pass",null); - HttpAPIConfig sunriseConfig = new HttpAPIConfig("https://api.sunrise-sunset.org/", "get", null, null, null, null, null); + HttpAPIConfig sunriseConfig = new HttpAPIConfig("https://api.sunrise-sunset.org/", "GET", null, null, null, null, null); HttpAPIConfig stockConfig = new HttpAPIConfig("https://api.worldtradingdata.com/api/v1/stock?symbol=SNAP,TWTR,VOD" + ".L&api_token=zuHlu2vZaehdZN6GmJdTiVlp7xgZn6gl6sfgmI4G6TY4ej0NLOzvy0TUl4D4", "get", null, null, null, null, null); - HttpAPIConfig mockPostConfig = new HttpAPIConfig("http://localhost:8091/", "post", headers, null, null, null,"key1=value1\nkey2=value2"); + HttpAPIConfig mockPostConfig = new HttpAPIConfig("http://localhost:8091/", "POST", headers, null, null, null,"key1=value1\nkey2=value2"); Map configs = new HashMap<>(); configs.put("stock", stockConfig); diff --git a/contrib/storage-http/src/test/resources/data/response.json b/contrib/storage-http/src/test/resources/data/response.json new file mode 100644 index 00000000000..bde2d5bf6d5 --- /dev/null +++ b/contrib/storage-http/src/test/resources/data/response.json @@ -0,0 +1,14 @@ +{"results": + {"sunrise":"6:13:58 AM", + "sunset":"5:59:55 PM", + "solar_noon":"12:06:56 PM", + "day_length":"11:45:57", + "civil_twilight_begin":"5:48:14 AM", + "civil_twilight_end":"6:25:38 PM", + "nautical_twilight_begin":"5:18:16 AM", + "nautical_twilight_end":"6:55:36 PM", + "astronomical_twilight_begin":"4:48:07 AM", + "astronomical_twilight_end":"7:25:45 PM" + }, + "status":"OK" +} diff --git a/contrib/storage-http/src/test/resources/logback-test.xml b/contrib/storage-http/src/test/resources/logback-test.xml.bak similarity index 100% rename from contrib/storage-http/src/test/resources/logback-test.xml rename to contrib/storage-http/src/test/resources/logback-test.xml.bak diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/util/DirectoryUtils.java b/exec/java-exec/src/main/java/org/apache/drill/exec/util/DirectoryUtils.java new file mode 100644 index 00000000000..02b7bdfcff6 --- /dev/null +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/util/DirectoryUtils.java @@ -0,0 +1,47 @@ +/* + * 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.drill.exec.util; + +import org.apache.drill.exec.ExecConstants; +import org.apache.drill.exec.ops.FragmentContext; + +public class DirectoryUtils { + + /** + * Returns a path to Drill's temp directory. First checks to see if the variable DRILL_TMP_DIR is + * set in the Drill configuration. If it is not, check the System environment to see if it is set there. + * If it is set in neither location, returns empty string. + * @param context + * @return Path of Drill's temp directory + * @throws Exception If the option is not set, or if there are some other issues, throw an exception + */ + public static String getDrillTempDirectory(FragmentContext context) throws Exception { + String drillTempDir; + + if (context.getOptions().getOption(ExecConstants.DRILL_TMP_DIR) != null) { + drillTempDir = context + .getOptions() + .getOption(ExecConstants.DRILL_TMP_DIR) + .string_val; + } else { + drillTempDir = System.getenv("DRILL_TMP_DIR"); + } + return drillTempDir; + } +}