You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/06/30 06:04:33 UTC

[GitHub] [ozone] umamaheswararao commented on a diff in pull request #3565: HDDS-6889. EC: put key command with EC replication can use ReplicationConfig validator

umamaheswararao commented on code in PR #3565:
URL: https://github.com/apache/ozone/pull/3565#discussion_r910637395


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -863,13 +865,35 @@ private void verifyReplication(String volumeName, String bucketName,
     }
   }
 
+  @ParameterizedTest
+  @ValueSource(strings = {"rs-3-3-1024k", "xor-3-5-2048k"})

Review Comment:
   You should pass valid values also? probably we can pass Pairs indicating valid config or not. Based valid flag, you can assert differently.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -863,13 +865,35 @@ private void verifyReplication(String volumeName, String bucketName,
     }
   }
 
+  @ParameterizedTest
+  @ValueSource(strings = {"rs-3-3-1024k", "xor-3-5-2048k"})
+  public void testPutKeyWithInvalidReplicationConfig(
+          String replicationValue) throws IOException {
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+
+    String value = "dummy";
+    store.createVolume(volumeName);
+    OzoneVolume volume = store.getVolume(volumeName);
+    volume.createBucket(bucketName);
+    OzoneBucket bucket = volume.getBucket(bucketName);
+
+    String keyName = UUID.randomUUID().toString();
+    ReplicationConfig replicationConfig =
+            new ECReplicationConfig(replicationValue);
+    Assertions.assertThrows(IllegalArgumentException.class,
+            () -> bucket.createKey(keyName,
+            value.getBytes(UTF_8).length, replicationConfig, new HashMap<>()));
+  }
+
   @Test
   public void testPutKey() throws IOException {
     String volumeName = UUID.randomUUID().toString();
     String bucketName = UUID.randomUUID().toString();
     Instant testStartTime = Instant.now();
 
     String value = "sample value";

Review Comment:
   Removed the unnecessary empty line



##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfigValidator.java:
##########
@@ -53,7 +54,8 @@ public ReplicationConfig validate(ReplicationConfig replicationConfig) {
       if (!validationRegexp.matcher(
           replicationConfig.configFormat()).matches()) {
         String replication = replicationConfig.getReplication();
-        if (replicationConfig instanceof ECReplicationConfig) {

Review Comment:
   Type is enum. We should be able to use == ?



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -863,13 +865,35 @@ private void verifyReplication(String volumeName, String bucketName,
     }
   }
 
+  @ParameterizedTest
+  @ValueSource(strings = {"rs-3-3-1024k", "xor-3-5-2048k"})
+  public void testPutKeyWithInvalidReplicationConfig(
+          String replicationValue) throws IOException {
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+
+    String value = "dummy";

Review Comment:
   Why do you need string 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