jnturton commented on code in PR #2650:
URL: https://github.com/apache/drill/pull/2650#discussion_r973710035


##########
contrib/storage-druid/src/main/java/org/apache/drill/exec/store/druid/rest/RestClientWrapper.java:
##########
@@ -17,38 +17,36 @@
  */
 package org.apache.drill.exec.store.druid.rest;
 
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.entity.StringEntity;
-import org.apache.http.impl.client.DefaultHttpClient;
-
-import javax.ws.rs.core.HttpHeaders;
-import java.io.IOException;
-import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.RequestBody;
+import okhttp3.Response;
 
-import static javax.ws.rs.core.MediaType.APPLICATION_JSON;
-import static org.apache.http.protocol.HTTP.CONTENT_TYPE;
+import java.nio.charset.StandardCharsets;
+import java.io.IOException;
 
 public class RestClientWrapper implements RestClient {
-  private static final HttpClient httpClient = new DefaultHttpClient();
-  private static final Charset DEFAULT_ENCODING = StandardCharsets.UTF_8;
+  // OkHttp client is designed to be shared across threads.
+  private final OkHttpClient httpClient = new OkHttpClient();
+
+  public Response get(String url) throws IOException {
+    Request get = new Request.Builder()
+      .url(url)
+      .addHeader("Content-Type", "application/json")

Review Comment:
   @pjfanning: also a valid question, but remember that this PR was a thread 
safety fix intended for backporting to stable so I strove to disturb as little 
possible while fixing the bug. Since the plugin specified this content-type 
here before, and we have no bug report related to it doing so, it continues to 
do that after this PR.
   
   Maybe when @cgivre sends in the EVF version of this plugin we can see if 
Druid is indeed happy for this content-type header to disappear. I'm almost 
sure you're right and that will be the case.



-- 
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

Reply via email to