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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to