[jira] [Commented] (HBASE-16267) Remove commons-httpclient dependency from hbase-rest module

[Josh Elser (JIRA)]

    [ 
https://issues.apache.org/jira/browse/HBASE-16267?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15389952#comment-15389952
 ] 

Josh Elser commented on HBASE-16267:
------------------------------------

Looks pretty good, [~tedyu]. Some quick comments.

Nit, commented code:

{code}
@@ -269,11 +291,11 @@ public class Client {
    */
   public Response head(Cluster cluster, String path, Header[] headers)
       throws IOException {
-    HeadMethod method = new HeadMethod();
+    HttpHead method = new HttpHead(path);
     try {
-      int code = execute(cluster, method, null, path);
-      headers = method.getResponseHeaders();
-      return new Response(code, headers, null);
+      HttpResponse resp = execute(cluster, method, null, path);
+      // headers = method.getResponseHeaders();
+      return new Response(resp.getStatusLine().getStatusCode(), 
resp.getAllHeaders(), null);
     } finally {
       method.releaseConnection();
     }
{code}

Document that this is not thread safe?

{code}
@@ -69,13 +73,14 @@ public class Response {
    * Constructor
    * @param code the HTTP response code
    * @param headers headers the HTTP response headers
-   * @param body the response body, can be null
+   * @param resp the response
    * @param in Inputstream if the response had one.
    */
-  public Response(int code, Header[] headers, byte[] body, InputStream in) {
+  public Response(int code, Header[] headers, HttpResponse resp, InputStream 
in) {
     this.code = code;
     this.headers = headers;
-    this.body = body;
+    this.body = null;
+    this.resp = resp;
     this.stream = in;
   }
 
@@ -129,6 +134,13 @@ public class Response {
    * @return the HTTP response body
    */
   public byte[] getBody() {
+    if (body == null) {
+      try {
+        body = Client.getResponseBody(resp);
+      } catch (IOException ioe) {
+        Log.debug("encountered ioe when obtaining body", ioe);
+      }
+    }
     return body;
   }
{code}

Isn't this exclusion already covered by the exclusions you added to 
dependencyManagement in the root-level pom.xml in the hadoop-3.0 profile?

{code}
@@ -388,6 +392,12 @@
         <dependency>
           <groupId>org.apache.hadoop</groupId>
           <artifactId>hadoop-common</artifactId>
+          <exclusions>
+            <exclusion>
+              <groupId>commons-httpclient</groupId>
+              <artifactId>commons-httpclient</artifactId>
+            </exclusion>
+          </exclusions>
         </dependency>
         <dependency>
           <groupId>org.apache.hadoop</groupId>
{code}

> Remove commons-httpclient dependency from hbase-rest module
> -----------------------------------------------------------
>
>                 Key: HBASE-16267
>                 URL: https://issues.apache.org/jira/browse/HBASE-16267
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Ted Yu
>            Assignee: Ted Yu
>             Fix For: 2.0.0
>
>         Attachments: 16267.v10.txt, 16267.v11.txt, 16267.v2.txt, 
> 16267.v4.txt, 16267.v6.txt, 16267.v8.txt, 16267.v9.txt
>
>
> hbase-rest module still has imports from org.apache.commons.httpclient .
> There is more work to be done after HBASE-15767 was integrated.
> In master branch, there seems to be transitive dependency which allows the 
> code to compile:
> {code}
> [INFO] +- org.apache.hadoop:hadoop-common:jar:2.7.1:compile
> [INFO] |  +- org.apache.hadoop:hadoop-annotations:jar:2.7.1:compile
> [INFO] |  +- commons-cli:commons-cli:jar:1.2:compile
> [INFO] |  +- org.apache.commons:commons-math3:jar:3.1.1:compile
> [INFO] |  +- xmlenc:xmlenc:jar:0.52:compile
> [INFO] |  +- commons-httpclient:commons-httpclient:jar:3.1:compile
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)