adoroszlai commented on code in PR #6405:
URL: https://github.com/apache/ozone/pull/6405#discussion_r1531763155


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/OzoneFileSystemTests.java:
##########
@@ -95,4 +98,20 @@ private static void listStatusIterator(FileSystem subject,
 
     assertEquals(total, iCount);
   }
+
+  static void createKeyWithECReplicationConfiguration(OzoneConfiguration conf, 
Path keyPath) throws IOException {

Review Comment:
   Let's avoid changing the input config by creating a new one:
   
   ```suggestion
     static void createKeyWithECReplicationConfiguration(OzoneConfiguration 
inputConf, Path keyPath) throws IOException {
       OzoneConfiguration conf = new OzoneConfiguration(inputConf);
     }
   ```



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/OzoneFileSystemTests.java:
##########
@@ -95,4 +98,20 @@ private static void listStatusIterator(FileSystem subject,
 
     assertEquals(total, iCount);
   }
+
+  static void createKeyWithECReplicationConfiguration(OzoneConfiguration conf, 
Path keyPath) throws IOException {
+    conf.set(OZONE_REPLICATION, "rs-3-2-1024k");
+    conf.set(OZONE_REPLICATION_TYPE, "EC");
+    URI uri = FileSystem.getDefaultUri(conf);
+    conf.setBoolean(
+        String.format("fs.%s.impl.disable.cache", uri.getScheme()), true);
+    FileSystem fileSystem = FileSystem.get(uri, conf);
+    ContractTestUtils.touch(fileSystem, keyPath);
+
+    // Refresh the cache of FileSystem
+    conf.unset(OZONE_REPLICATION);
+    conf.unset(OZONE_REPLICATION_TYPE);
+    fileSystem = FileSystem.get(uri, conf);
+    fileSystem.close();

Review Comment:
   No longer needed, as the original config is unchanged.
   
   ```suggestion
   ```



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/OzoneFileSystemTests.java:
##########
@@ -95,4 +98,20 @@ private static void listStatusIterator(FileSystem subject,
 
     assertEquals(total, iCount);
   }
+
+  static void createKeyWithECReplicationConfiguration(OzoneConfiguration conf, 
Path keyPath) throws IOException {
+    conf.set(OZONE_REPLICATION, "rs-3-2-1024k");
+    conf.set(OZONE_REPLICATION_TYPE, "EC");
+    URI uri = FileSystem.getDefaultUri(conf);
+    conf.setBoolean(
+        String.format("fs.%s.impl.disable.cache", uri.getScheme()), true);
+    FileSystem fileSystem = FileSystem.get(uri, conf);
+    ContractTestUtils.touch(fileSystem, keyPath);

Review Comment:
   ```suggestion
       try (FileSystem fileSystem = FileSystem.get(uri, conf)) {
         ContractTestUtils.touch(fileSystem, keyPath);
         // add here the assertions about the new key
       }
   ```



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java:
##########
@@ -325,6 +326,19 @@ void testCreateDoesNotAddParentDirKeys() throws Exception {
     fs.delete(grandparent, true);
   }
 
+  @Test
+  public void testCreateKeyWithECReplicationConfig() throws Exception {
+    String testKeyName = "testKey";
+    Path testKeyPath = new Path(bucketPath, testKeyName);
+    createKeyWithECReplicationConfiguration(cluster.getConf(), testKeyPath);
+
+    OzoneKeyDetails key = getKey(testKeyPath, false);
+    assertEquals(HddsProtos.ReplicationType.EC,
+        key.getReplicationConfig().getReplicationType());
+    assertEquals("rs-3-2-1024k",
+        key.getReplicationConfig().getReplication());

Review Comment:
   These assertions are still duplicated between the test classes.  Please move 
these to:
   
   
https://github.com/apache/ozone/blob/4a4a7d5a4a43af1633460449538779797abe67ac/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/OzoneFileSystemTests.java#L102



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

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