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


##########
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")
+      .build();
 
-  public HttpResponse get(String url) throws IOException {
-    HttpGet httpget = new HttpGet(url);
-    httpget.addHeader(HttpHeaders.CONTENT_TYPE, APPLICATION_JSON);
-    return httpClient.execute(httpget);
+    return httpClient.newCall(get).execute();
   }
 
-  public HttpResponse post(String url, String body) throws IOException {
-    HttpPost httppost = new HttpPost(url);
-    httppost.addHeader(CONTENT_TYPE, APPLICATION_JSON);
-    HttpEntity entity = new StringEntity(body, DEFAULT_ENCODING);
-    httppost.setEntity(entity);
+  public Response post(String url, String body) throws IOException {
+    RequestBody postBody = 
RequestBody.create(body.getBytes(StandardCharsets.UTF_8));

Review Comment:
   > Would it be possible to use the RequestBody.create method that takes a 
String instead of converting to a byte[]?
   > 
   > https://square.github.io/okhttp/#post-to-a-server
   
   Ugh I missed that. @cgivre please may we incorporate @pjfanning's suggestion 
here the EVF conversion PR.



##########
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")
+      .build();
 
-  public HttpResponse get(String url) throws IOException {
-    HttpGet httpget = new HttpGet(url);
-    httpget.addHeader(HttpHeaders.CONTENT_TYPE, APPLICATION_JSON);
-    return httpClient.execute(httpget);
+    return httpClient.newCall(get).execute();
   }
 
-  public HttpResponse post(String url, String body) throws IOException {
-    HttpPost httppost = new HttpPost(url);
-    httppost.addHeader(CONTENT_TYPE, APPLICATION_JSON);
-    HttpEntity entity = new StringEntity(body, DEFAULT_ENCODING);
-    httppost.setEntity(entity);
+  public Response post(String url, String body) throws IOException {
+    RequestBody postBody = 
RequestBody.create(body.getBytes(StandardCharsets.UTF_8));

Review Comment:
   > Would it be possible to use the RequestBody.create method that takes a 
String instead of converting to a byte[]?
   > 
   > https://square.github.io/okhttp/#post-to-a-server
   
   Ugh I missed that. @cgivre please may we incorporate @pjfanning's suggestion 
here in the EVF conversion PR.



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