steveloughran commented on a change in pull request #3412:
URL: https://github.com/apache/hadoop/pull/3412#discussion_r707201841



##########
File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -44,21 +44,21 @@ public void testEncryptionAlgorithmSetToDES() throws 
Throwable {
     assumeEnabled();
     intercept(IOException.class, "Unknown Server Side algorithm DES", () -> {
 
-        Configuration conf = super.createConfiguration();
-        //DES is an invalid encryption algorithm
-        conf.set(Constants.S3_ENCRYPTION_ALGORITHM, "DES");
-        S3AContract contract = (S3AContract) createContract(conf);
-        contract.init();
-        //extract the test FS
-        FileSystem fileSystem = contract.getTestFileSystem();
-        assertNotNull("null filesystem", fileSystem);
-        URI fsURI = fileSystem.getUri();
-        LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
-        assertEquals("wrong filesystem of " + fsURI,
-            contract.getScheme(), fsURI.getScheme());
-        fileSystem.initialize(fsURI, conf);
-        throw new Exception("Do not reach here");
-      });
+      Configuration conf = super.createConfiguration();
+      //DES is an invalid encryption algorithm
+      conf.set(Constants.S3_ENCRYPTION_ALGORITHM, "DES");
+      S3AContract contract = (S3AContract) createContract(conf);
+      contract.init();
+      //extract the test FS
+      FileSystem fileSystem = contract.getTestFileSystem();
+      assertNotNull("null filesystem", fileSystem);
+      URI fsURI = fileSystem.getUri();
+      LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+      assertEquals("wrong filesystem of " + fsURI,
+          contract.getScheme(), fsURI.getScheme());
+      fileSystem.initialize(fsURI, conf);
+      throw new Exception("Do not reach here");

Review comment:
       can you just return `fileSystem`

##########
File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -44,21 +44,21 @@ public void testEncryptionAlgorithmSetToDES() throws 
Throwable {
     assumeEnabled();
     intercept(IOException.class, "Unknown Server Side algorithm DES", () -> {
 
-        Configuration conf = super.createConfiguration();
-        //DES is an invalid encryption algorithm
-        conf.set(Constants.S3_ENCRYPTION_ALGORITHM, "DES");
-        S3AContract contract = (S3AContract) createContract(conf);
-        contract.init();
-        //extract the test FS
-        FileSystem fileSystem = contract.getTestFileSystem();
-        assertNotNull("null filesystem", fileSystem);
-        URI fsURI = fileSystem.getUri();
-        LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
-        assertEquals("wrong filesystem of " + fsURI,
-            contract.getScheme(), fsURI.getScheme());
-        fileSystem.initialize(fsURI, conf);
-        throw new Exception("Do not reach here");
-      });
+      Configuration conf = super.createConfiguration();

Review comment:
       nit; if we generate that error "unknown server side algorithm", should 
we change to "unknown s3 encryption algorithm"

##########
File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -119,24 +119,24 @@ public void 
testEncryptionAlgorithmSSES3WithEncryptionKey() throws
     assumeEnabled();
     intercept(IOException.class, S3AUtils.SSE_S3_WITH_KEY_ERROR, () -> {
 
-        Configuration conf = super.createConfiguration();
-        //SSE-S3 cannot be configured with an encryption key
-        conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
-            S3AEncryptionMethods.SSE_S3.getMethod());
-        conf.set(Constants.S3_ENCRYPTION_KEY,
-            "4niV/jPK5VFRHY+KNb6wtqYd4xXyMgdJ9XQJpcQUVbs=");
-        S3AContract contract = (S3AContract) createContract(conf);
-        contract.init();
-        //extract the test FS
-        FileSystem fileSystem = contract.getTestFileSystem();
-        assertNotNull("null filesystem", fileSystem);
-        URI fsURI = fileSystem.getUri();
-        LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
-        assertEquals("wrong filesystem of " + fsURI,
-            contract.getScheme(), fsURI.getScheme());
-        fileSystem.initialize(fsURI, conf);
-        throw new Exception("Do not reach here");
-      });
+      Configuration conf = super.createConfiguration();
+      //SSE-S3 cannot be configured with an encryption key
+      conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
+          S3AEncryptionMethods.SSE_S3.getMethod());
+      conf.set(Constants.S3_ENCRYPTION_KEY,
+          "4niV/jPK5VFRHY+KNb6wtqYd4xXyMgdJ9XQJpcQUVbs=");
+      S3AContract contract = (S3AContract) createContract(conf);
+      contract.init();
+      //extract the test FS
+      FileSystem fileSystem = contract.getTestFileSystem();
+      assertNotNull("null filesystem", fileSystem);
+      URI fsURI = fileSystem.getUri();
+      LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+      assertEquals("wrong filesystem of " + fsURI,
+          contract.getScheme(), fsURI.getScheme());
+      fileSystem.initialize(fsURI, conf);
+      throw new Exception("Do not reach here");

Review comment:
       same

##########
File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -69,47 +69,47 @@ public void 
testEncryptionAlgorithmSSECWithNoEncryptionKey() throws
     intercept(IllegalArgumentException.class, "The value of property " +
         Constants.S3_ENCRYPTION_KEY + " must not be null", () -> {
 
-        Configuration conf = super.createConfiguration();
-        //SSE-C must be configured with an encryption key
-        conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
-            S3AEncryptionMethods.SSE_C.getMethod());
-        conf.set(Constants.S3_ENCRYPTION_KEY, null);
-        S3AContract contract = (S3AContract) createContract(conf);
-        contract.init();
-        //extract the test FS
-        FileSystem fileSystem = contract.getTestFileSystem();
-        assertNotNull("null filesystem", fileSystem);
-        URI fsURI = fileSystem.getUri();
-        LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
-        assertEquals("wrong filesystem of " + fsURI,
-            contract.getScheme(), fsURI.getScheme());
-        fileSystem.initialize(fsURI, conf);
-        throw new Exception("Do not reach here");
-      });
+      Configuration conf = super.createConfiguration();
+      //SSE-C must be configured with an encryption key
+      conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
+          S3AEncryptionMethods.SSE_C.getMethod());
+      conf.set(Constants.S3_ENCRYPTION_KEY, null);
+      S3AContract contract = (S3AContract) createContract(conf);
+      contract.init();
+      //extract the test FS
+      FileSystem fileSystem = contract.getTestFileSystem();
+      assertNotNull("null filesystem", fileSystem);
+      URI fsURI = fileSystem.getUri();
+      LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+      assertEquals("wrong filesystem of " + fsURI,
+          contract.getScheme(), fsURI.getScheme());
+      fileSystem.initialize(fsURI, conf);
+      throw new Exception("Do not reach here");

Review comment:
       again, return fileSystem

##########
File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -69,47 +69,47 @@ public void 
testEncryptionAlgorithmSSECWithNoEncryptionKey() throws
     intercept(IllegalArgumentException.class, "The value of property " +
         Constants.S3_ENCRYPTION_KEY + " must not be null", () -> {
 
-        Configuration conf = super.createConfiguration();
-        //SSE-C must be configured with an encryption key
-        conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
-            S3AEncryptionMethods.SSE_C.getMethod());
-        conf.set(Constants.S3_ENCRYPTION_KEY, null);
-        S3AContract contract = (S3AContract) createContract(conf);
-        contract.init();
-        //extract the test FS
-        FileSystem fileSystem = contract.getTestFileSystem();
-        assertNotNull("null filesystem", fileSystem);
-        URI fsURI = fileSystem.getUri();
-        LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
-        assertEquals("wrong filesystem of " + fsURI,
-            contract.getScheme(), fsURI.getScheme());
-        fileSystem.initialize(fsURI, conf);
-        throw new Exception("Do not reach here");
-      });
+      Configuration conf = super.createConfiguration();
+      //SSE-C must be configured with an encryption key
+      conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
+          S3AEncryptionMethods.SSE_C.getMethod());
+      conf.set(Constants.S3_ENCRYPTION_KEY, null);
+      S3AContract contract = (S3AContract) createContract(conf);
+      contract.init();
+      //extract the test FS
+      FileSystem fileSystem = contract.getTestFileSystem();
+      assertNotNull("null filesystem", fileSystem);
+      URI fsURI = fileSystem.getUri();
+      LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+      assertEquals("wrong filesystem of " + fsURI,
+          contract.getScheme(), fsURI.getScheme());
+      fileSystem.initialize(fsURI, conf);
+      throw new Exception("Do not reach here");
+    });
   }
 
   @Test
   public void testEncryptionAlgorithmSSECWithBlankEncryptionKey() throws
     Throwable {
     intercept(IOException.class, S3AUtils.SSE_C_NO_KEY_ERROR, () -> {
 
-        Configuration conf = super.createConfiguration();
-        //SSE-C must be configured with an encryption key
-        conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
-            S3AEncryptionMethods.SSE_C.getMethod());
-        conf.set(Constants.S3_ENCRYPTION_KEY, "");
-        S3AContract contract = (S3AContract) createContract(conf);
-        contract.init();
-        //extract the test FS
-        FileSystem fileSystem = contract.getTestFileSystem();
-        assertNotNull("null filesystem", fileSystem);
-        URI fsURI = fileSystem.getUri();
-        LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
-        assertEquals("wrong filesystem of " + fsURI,
-            contract.getScheme(), fsURI.getScheme());
-        fileSystem.initialize(fsURI, conf);
-        throw new Exception("Do not reach here");
-      });
+      Configuration conf = super.createConfiguration();
+      //SSE-C must be configured with an encryption key
+      conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
+          S3AEncryptionMethods.SSE_C.getMethod());
+      conf.set(Constants.S3_ENCRYPTION_KEY, "");
+      S3AContract contract = (S3AContract) createContract(conf);
+      contract.init();
+      //extract the test FS
+      FileSystem fileSystem = contract.getTestFileSystem();
+      assertNotNull("null filesystem", fileSystem);
+      URI fsURI = fileSystem.getUri();
+      LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+      assertEquals("wrong filesystem of " + fsURI,
+          contract.getScheme(), fsURI.getScheme());
+      fileSystem.initialize(fsURI, conf);
+      throw new Exception("Do not reach here");

Review comment:
       &same




-- 
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