Re: [PR] HDDS-9644. Fixed incorrect validation of path with namespace du commands. [ozone]

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

devmadhuu commented on code in PR #6755:
URL: https://github.com/apache/ozone/pull/6755#discussion_r1626585480


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestNSSummaryAdmin.java:
##########
@@ -115,7 +115,25 @@ public void testNSSummaryCLIRoot() throws UnsupportedEncodingException {
     String path = "/";
     executeAdminCommands(path);
     // Should throw warning - only buckets can have bucket layout.
-    assertThat(getOutContentString()).contains("[Warning] Namespace CLI is not designed for OBS bucket layout.");
+    assertThat(getOutContentString()).doesNotContain("INVALID_VOLUME_NAME");
+    assertThat(getOutContentString()).doesNotContain(
+        "[Warning] Namespace CLI is not designed for OBS bucket layout.");
+    assertThat(getOutContentString()).contains("Put more files into it to visualize DU");
+    assertThat(getOutContentString()).contains("Put more files into it to visualize file size distribution");
+  }
+
+  /**
+   * Test NSSummaryCLI on volume.
+   */
+  @Test
+  public void testNSSummaryCLIVolume() throws UnsupportedEncodingException {
+    // Running on root path.
+    String path = "/" + volumeName;
+    executeAdminCommands(path);
+    // Should throw warning - only buckets can have bucket layout.
+    assertThat(getOutContentString()).doesNotContain("INVALID_BUCKET_NAME");
+    assertThat(getOutContentString()).doesNotContain(
+        "[Warning] Namespace CLI is not designed for OBS bucket layout.");

Review Comment:
   Yes , we should not validate the layout of bucket now.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/nssummary/NSSummaryAdmin.java:
##########
@@ -115,6 +115,11 @@ public boolean isNotValidBucketOrOBSBucket(String path) {
         OzoneConfiguration.of(getOzoneConfig()));
     try (OzoneClient ozoneClient = OzoneClientFactory.getRpcClient(getOzoneConfig())) {
       ObjectStore objectStore = ozoneClient.getObjectStore();
+      // Return false if path is root, just a volume

Review Comment:
   Comment is confusing. I meant, if path is root, and no volume or bucket name is present in path, then what we are trying to say in comment with "`just a volume`" , just "`Return false if path is root`" should have been enough. Am i missing something here ?



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