[GitHub] [hadoop] ayushtkn commented on a diff in pull request #5660: HDFS-17014. HttpFS Add Support getStatus API

["ayushtkn (via GitHub)" <gi...@apache.org>]

ayushtkn commented on code in PR #5660:
URL: https://github.com/apache/hadoop/pull/5660#discussion_r1198455422


##########
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java:
##########
@@ -208,6 +210,9 @@ public static FILE_TYPE getType(FileStatus fileStatus) {
   public static final String XATTRNAMES_JSON = "XAttrNames";
   public static final String ECPOLICY_JSON = "ecPolicyObj";
   public static final String SYMLINK_JSON = "symlink";
+  public static final String CAPACITY_JSON="capacity";
+  public static final String USED_JSON="used";
+  public static final String REMAINING_JSON="remaining";

Review Comment:
   space required before and after '='



##########
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/client/HttpFSFileSystem.java:
##########
@@ -1756,6 +1762,18 @@ public FileStatus getFileLinkStatus(final Path path) throws IOException {
     return status.makeQualified(getUri(), path);
   }
 
+  @Override
+  public FsStatus getStatus(final Path path) throws IOException {
+    Map<String, String> params = new HashMap<>();
+    params.put(OP_PARAM, Operation.GETSTATUS.toString());
+    HttpURLConnection conn =
+        getConnection(Operation.GETSTATUS.getMethod(), params, path, true);
+    HttpExceptionUtils.validateResponse(conn, HttpURLConnection.HTTP_OK);
+    JSONObject json = (JSONObject) HttpFSUtils.jsonParse(conn);
+    FsStatus status = JsonUtilClient.toFsStatus(json);
+    return status;

Review Comment:
   inline return
   ```
       return JsonUtilClient.toFsStatus(json);
   ```



##########
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/client/BaseTestHttpFSWith.java:
##########
@@ -2081,6 +2085,33 @@ private void testGetFileLinkStatus() throws Exception {
     assertTrue(fs.getFileLinkStatus(linkToFile).isSymlink());
   }
 
+  private void testGetStatus() throws Exception {
+    if (isLocalFS()) {
+      // do not test the getStatus for local FS.
+      return;
+    }
+    final Path path = new Path("/foo");
+    FileSystem fs = FileSystem.get(path.toUri(), this.getProxiedFSConf());
+    if(fs instanceof DistributedFileSystem){
+      DistributedFileSystem dfs = (DistributedFileSystem)
+          FileSystem.get(path.toUri(), this.getProxiedFSConf());
+      FileSystem httpFs = this.getHttpFSFileSystem();
+
+      FsStatus dfsFsStatus = dfs.getStatus(path);
+      FsStatus httpFsStatus = httpFs.getStatus(path);
+
+      //Validate used free and capacity are the same as DistributedFileSystem
+      Assert.assertEquals(dfsFsStatus.getUsed(), httpFsStatus.getUsed());
+      Assert.assertEquals(dfsFsStatus.getRemaining(),
+          httpFsStatus.getRemaining());
+      Assert.assertEquals(dfsFsStatus.getCapacity(), httpFsStatus.getCapacity());

Review Comment:
   There are already static imports for ``assertEquals`` you don't need the ``Assert`` prefix



##########
hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/client/BaseTestHttpFSWith.java:
##########
@@ -2081,6 +2085,33 @@ private void testGetFileLinkStatus() throws Exception {
     assertTrue(fs.getFileLinkStatus(linkToFile).isSymlink());
   }
 
+  private void testGetStatus() throws Exception {
+    if (isLocalFS()) {
+      // do not test the getStatus for local FS.
+      return;
+    }
+    final Path path = new Path("/foo");
+    FileSystem fs = FileSystem.get(path.toUri(), this.getProxiedFSConf());
+    if(fs instanceof DistributedFileSystem){
+      DistributedFileSystem dfs = (DistributedFileSystem)
+          FileSystem.get(path.toUri(), this.getProxiedFSConf());

Review Comment:
   missing space & code ain't formatted properly
   ```
       if (fs instanceof DistributedFileSystem) {
         DistributedFileSystem dfs =
             (DistributedFileSystem) FileSystem.get(path.toUri(), this.getProxiedFSConf());
         FileSystem httpFs = this.getHttpFSFileSystem();
   ```



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

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


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