[GitHub] [ozone] ayushtkn commented on a change in pull request #1723: HDDS-4588. If volume's quota is enabled then bucket's quota cannot be cleared

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

ayushtkn commented on a change in pull request #1723:
URL: https://github.com/apache/ozone/pull/1723#discussion_r547082444



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -311,6 +311,23 @@ public void testSetAndClrQuota() throws IOException {
         ozoneBucket.getQuotaInBytes());
     Assert.assertEquals(1000L, ozoneBucket.getQuotaInNamespace());
 
+    try {
+      ozoneBucket.clearSpaceQuota();
+      Assert.fail("clear quota should be failed");
+    } catch (IOException ex) {
+      GenericTestUtils.assertExceptionContains("Before clear bucket " +
+          "quotaInBytes, volume quotaInBytes need to be clear first.", ex);
+    }

Review comment:
       Can try something like this -
   ```
     LambdaTestUtils.intercept(IOException.class, "Before clear bucket "
               + "quotaInBytes, volume quotaInBytes need to be clear first.",
           () -> ozoneBucket.clearSpaceQuota());
   ```
   Shall save lines and looks better

##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -311,6 +311,23 @@ public void testSetAndClrQuota() throws IOException {
         ozoneBucket.getQuotaInBytes());
     Assert.assertEquals(1000L, ozoneBucket.getQuotaInNamespace());
 
+    try {
+      ozoneBucket.clearSpaceQuota();
+      Assert.fail("clear quota should be failed");
+    } catch (IOException ex) {
+      GenericTestUtils.assertExceptionContains("Before clear bucket " +
+          "quotaInBytes, volume quotaInBytes need to be clear first.", ex);
+    }
+
+    try {
+      ozoneBucket.clearCountQuota();
+      Assert.fail("clear quota should be failed");
+    } catch (IOException ex) {
+      GenericTestUtils.assertExceptionContains("Before clear bucket " +
+          "quotaInNamespace, volume quotaInNamespace need to be clear first.",
+          ex);
+    }

Review comment:
       Similarly here -
   ```
       LambdaTestUtils.intercept(IOException.class, "Before clear bucket "
               + "quotaInNamespace, volume quotaInNamespace need to be clear first.",
           () -> ozoneBucket.clearCountQuota());
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java
##########
@@ -259,9 +266,16 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
   }
 
   public boolean checkQuotaNamespaceValid(OmVolumeArgs omVolumeArgs,
-      OmBucketArgs omBucketArgs) {
+      OmBucketArgs omBucketArgs) throws IOException{
     long quotaInNamespace = omBucketArgs.getQuotaInNamespace();
 
+    if (quotaInNamespace == OzoneConsts.QUOTA_RESET &&
+        omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+      throw new OMException("Before clear bucket " +
+          "quotaInNamespace, volume quotaInNamespace need to be clear first.",

Review comment:
       Seems some grammatical  error in the message, Can you check and reframe if possible, 
   May be something like a warning, Can not clear bucket namespace quota because volume namespace quota is not unset. Or may be something better....




----------------------------------------------------------------
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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org