[GitHub] [hadoop-ozone] cxorm commented on a change in pull request #1233: HDDS-3725. Ozone sh volume client support quota option.

[GitBox <gi...@apache.org>]

cxorm commented on a change in pull request #1233:
URL: https://github.com/apache/hadoop-ozone/pull/1233#discussion_r468426519



##########
File path: hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/TestJsonUtils.java
##########
@@ -32,11 +32,11 @@
 
   @Test
   public void printObjectAsJson() throws IOException {
-    OzoneQuota quota = new OzoneQuota(123, OzoneQuota.Units.MB);
+    OzoneQuota quota = OzoneQuota.parseQuota("123MB", 1000L);
 
     String result = JsonUtils.toJsonStringWithDefaultPrettyPrinter(quota);
 

Review comment:
       I think we should verify `quotaInCounts` here, too.

##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -271,10 +271,10 @@ public void testSetVolumeQuota()
       throws IOException {
     String volumeName = UUID.randomUUID().toString();
     store.createVolume(volumeName);
-    store.getVolume(volumeName).setQuota(
-        OzoneQuota.parseQuota("100000000 BYTES"));
+    store.getVolume(volumeName).setQuota(OzoneQuota.parseQuota("1GB", 0L));
     OzoneVolume volume = store.getVolume(volumeName);
-    Assert.assertEquals(100000000L, volume.getQuota());
+    Assert.assertEquals(1024 * 1024 * 1024,
+        volume.getQuotaInBytes());

Review comment:
       The same as above.
   We should verify two quota attributes if the `OzoneQuota` has two attributes.

##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java
##########
@@ -734,7 +734,8 @@ public void testTempMount() throws Exception {
     // Sanity check
     Assert.assertNull(volumeArgs.getOwner());
     Assert.assertNull(volumeArgs.getAdmin());
-    Assert.assertNull(volumeArgs.getQuota());
+    Assert.assertNull(volumeArgs.getQuotaInBytes());
+    Assert.assertEquals(0, volumeArgs.getQuotaInCounts());

Review comment:
       I think we should verify two quota attributes if the `OzoneQuota` has two attributes.

##########
File path: hadoop-ozone/dist/src/main/smoketest/topology/loaddata.robot
##########
@@ -25,7 +25,7 @@ Test Timeout        5 minutes
 
 *** Test Cases ***
 Create a volume, bucket and key
-    ${output} =         Execute          ozone sh volume create topvol1 --quota 100TB
+    ${output} =         Execute          ozone sh volume create topvol1

Review comment:
       Why could we not set `spaceQuota` 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.

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



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