You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/10/12 09:12:09 UTC

[GitHub] [ozone] dombizita opened a new pull request #2735: HDDS-5826 [HTTPFSGW] Remove or replace Hadoop shaded guava dependencies.

dombizita opened a new pull request #2735:
URL: https://github.com/apache/ozone/pull/2735


   ## What changes were proposed in this pull request?
   
   There are some shaded Guava dependencies coming from the Hadoop project's shaded Guava version, which could be easily replaced. So in this change I removed these imports from the classes and replaced them.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-5826
   
   ## How was this patch tested?
   
   Run the build manually with skip tests and the build was successful.
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] dombizita commented on a change in pull request #2735: HDDS-5826 [HTTPFSGW] Remove or replace Hadoop shaded guava dependencies.

Posted by GitBox <gi...@apache.org>.
dombizita commented on a change in pull request #2735:
URL: https://github.com/apache/ozone/pull/2735#discussion_r728225868



##########
File path: hadoop-ozone/httpfsgateway/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java
##########
@@ -1402,13 +1399,13 @@ public void setXAttr(Path f, String name, byte[] value,
     JSONObject json = (JSONObject) HttpFSUtils.jsonParse(conn);
     Map<String, byte[]> xAttrs = createXAttrMap(
         (JSONArray) json.get(XATTRS_JSON));
-    return xAttrs != null ? xAttrs.get(name) : null;
+    return xAttrs.get(name);

Review comment:
       My previous PR failed because the missing of this change, the findbug error was: "Redundant nullcheck of xAttrs, which is known to be non-null in org.apache.hadoop.fs.http.client.HttpFSFileSystem.getXAttr(Path, String) Redundant null check at HttpFSFileSystem.java:[line 1402]". 
   xAttrs is created in the line before with the createXAttrMap() method, which I changed in this PR, because it used Maps.newHashMap() to create xAttrs and I replaced it with a HashMap constructor call. Because of this change the null check is not neccessary, as the method couldn't return null.




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] fapifta commented on a change in pull request #2735: HDDS-5826 [HTTPFSGW] Remove or replace Hadoop shaded guava dependencies.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #2735:
URL: https://github.com/apache/ozone/pull/2735#discussion_r728118824



##########
File path: hadoop-ozone/httpfsgateway/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java
##########
@@ -1402,13 +1399,13 @@ public void setXAttr(Path f, String name, byte[] value,
     JSONObject json = (JSONObject) HttpFSUtils.jsonParse(conn);
     Map<String, byte[]> xAttrs = createXAttrMap(
         (JSONArray) json.get(XATTRS_JSON));
-    return xAttrs != null ? xAttrs.get(name) : null;
+    return xAttrs.get(name);

Review comment:
       Is there something here that ensures the xAttrs that have been read is not null, or is this accidental?




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] fapifta commented on a change in pull request #2735: HDDS-5826 [HTTPFSGW] Remove or replace Hadoop shaded guava dependencies.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #2735:
URL: https://github.com/apache/ozone/pull/2735#discussion_r728118824



##########
File path: hadoop-ozone/httpfsgateway/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java
##########
@@ -1402,13 +1399,13 @@ public void setXAttr(Path f, String name, byte[] value,
     JSONObject json = (JSONObject) HttpFSUtils.jsonParse(conn);
     Map<String, byte[]> xAttrs = createXAttrMap(
         (JSONArray) json.get(XATTRS_JSON));
-    return xAttrs != null ? xAttrs.get(name) : null;
+    return xAttrs.get(name);

Review comment:
       Is there something here that ensures the xAttrs read is not null, or is this accidental?




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] fapifta commented on a change in pull request #2735: HDDS-5826 [HTTPFSGW] Remove or replace Hadoop shaded guava dependencies.

Posted by GitBox <gi...@apache.org>.
fapifta commented on a change in pull request #2735:
URL: https://github.com/apache/ozone/pull/2735#discussion_r728118824



##########
File path: hadoop-ozone/httpfsgateway/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java
##########
@@ -1402,13 +1399,13 @@ public void setXAttr(Path f, String name, byte[] value,
     JSONObject json = (JSONObject) HttpFSUtils.jsonParse(conn);
     Map<String, byte[]> xAttrs = createXAttrMap(
         (JSONArray) json.get(XATTRS_JSON));
-    return xAttrs != null ? xAttrs.get(name) : null;
+    return xAttrs.get(name);

Review comment:
       Is there something here that ensures the xAttrs read is not null, or is this accidental?

##########
File path: hadoop-ozone/httpfsgateway/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java
##########
@@ -1402,13 +1399,13 @@ public void setXAttr(Path f, String name, byte[] value,
     JSONObject json = (JSONObject) HttpFSUtils.jsonParse(conn);
     Map<String, byte[]> xAttrs = createXAttrMap(
         (JSONArray) json.get(XATTRS_JSON));
-    return xAttrs != null ? xAttrs.get(name) : null;
+    return xAttrs.get(name);

Review comment:
       Is there something here that ensures the xAttrs that have been read is not null, or is this accidental?




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] fapifta commented on pull request #2735: HDDS-5826 [HTTPFSGW] Remove or replace Hadoop shaded guava dependencies.

Posted by GitBox <gi...@apache.org>.
fapifta commented on pull request #2735:
URL: https://github.com/apache/ozone/pull/2735#issuecomment-942362618


   Hi @dombizita,
   thank you for taking this on, the changes look good, apart from the one change I have added a comment to. If that is cleared, I think we can merge this to the branch.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] dombizita commented on a change in pull request #2735: HDDS-5826 [HTTPFSGW] Remove or replace Hadoop shaded guava dependencies.

Posted by GitBox <gi...@apache.org>.
dombizita commented on a change in pull request #2735:
URL: https://github.com/apache/ozone/pull/2735#discussion_r728225868



##########
File path: hadoop-ozone/httpfsgateway/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java
##########
@@ -1402,13 +1399,13 @@ public void setXAttr(Path f, String name, byte[] value,
     JSONObject json = (JSONObject) HttpFSUtils.jsonParse(conn);
     Map<String, byte[]> xAttrs = createXAttrMap(
         (JSONArray) json.get(XATTRS_JSON));
-    return xAttrs != null ? xAttrs.get(name) : null;
+    return xAttrs.get(name);

Review comment:
       My previous PR failed because the missing of this change, the findbug error was: "Redundant nullcheck of xAttrs, which is known to be non-null in org.apache.hadoop.fs.http.client.HttpFSFileSystem.getXAttr(Path, String) Redundant null check at HttpFSFileSystem.java:[line 1402]". 
   xAttrs is created in the line before with the createXAttrMap() method, which I changed in this PR, because it used Maps.newHashMap() to create xAttrs and I replaced it with a HashMap constructor call. Because of this change the null check is not neccessary, as the method couldn't return null.




-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] fapifta merged pull request #2735: HDDS-5826 [HTTPFSGW] Remove or replace Hadoop shaded guava dependencies.

Posted by GitBox <gi...@apache.org>.
fapifta merged pull request #2735:
URL: https://github.com/apache/ozone/pull/2735


   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] fapifta commented on pull request #2735: HDDS-5826 [HTTPFSGW] Remove or replace Hadoop shaded guava dependencies.

Posted by GitBox <gi...@apache.org>.
fapifta commented on pull request #2735:
URL: https://github.com/apache/ozone/pull/2735#issuecomment-942362618


   Hi @dombizita,
   thank you for taking this on, the changes look good, apart from the one change I have added a comment to. If that is cleared, I think we can merge this to the branch.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org