[ 
https://issues.apache.org/jira/browse/HADOOP-17823?focusedWorklogId=633503&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-633503
 ]

ASF GitHub Bot logged work on HADOOP-17823:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 04/Aug/21 12:42
            Start Date: 04/Aug/21 12:42
    Worklog Time Spent: 10m 
      Work Description: mukund-thakur commented on a change in pull request 
#3263:
URL: https://github.com/apache/hadoop/pull/3263#discussion_r682576434



##########
File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java
##########
@@ -229,12 +233,45 @@ public static FileContext 
createTestFileContext(Configuration conf)
     // make this whole class not run by default
     Assume.assumeTrue("No test filesystem in " + TEST_FS_S3A_NAME,
         liveTest);
+    // Skip if S3Guard and S3-CSE are enabled.
+    skipIfS3GuardAndS3CSEEnabled(conf);
     // patch in S3Guard options
     maybeEnableS3Guard(conf);
     FileContext fc = FileContext.getFileContext(testURI, conf);
     return fc;
   }
 
+  /**
+   * Skip if S3Guard and S3CSE are enabled together.
+   *
+   * @param conf Configuration used to verify if S3Guard and S3CSE are enabled.

Review comment:
       nit: just mention test configuration as you have already mentioned what 
this method does.

##########
File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java
##########
@@ -229,12 +233,45 @@ public static FileContext 
createTestFileContext(Configuration conf)
     // make this whole class not run by default
     Assume.assumeTrue("No test filesystem in " + TEST_FS_S3A_NAME,
         liveTest);
+    // Skip if S3Guard and S3-CSE are enabled.
+    skipIfS3GuardAndS3CSEEnabled(conf);
     // patch in S3Guard options
     maybeEnableS3Guard(conf);
     FileContext fc = FileContext.getFileContext(testURI, conf);
     return fc;
   }
 
+  /**
+   * Skip if S3Guard and S3CSE are enabled together.
+   *
+   * @param conf Configuration used to verify if S3Guard and S3CSE are enabled.
+   */
+  private static void skipIfS3GuardAndS3CSEEnabled(Configuration conf) {
+    String encryptionMethod =
+        conf.getTrimmed(SERVER_SIDE_ENCRYPTION_ALGORITHM, "");
+    String metaStore = conf.getTrimmed(S3_METADATA_STORE_IMPL, "");
+    if (encryptionMethod.equals(S3AEncryptionMethods.CSE_KMS.getMethod()) &&
+        !metaStore.equals(S3GUARD_METASTORE_NULL)) {
+      skip("Skipped if CSE is enabled with S3Guard.");
+    }
+  }
+
+  /**
+   * Either skip if PathIOE occurred due to S3CSE and S3Guard
+   * incompatibility or throw the PathIOE.
+   *
+   * @param ioe PathIOE being parsed.
+   * @throws PathIOException Throw the PathIOE if it doesn't relate to S3CSE

Review comment:
       nit: throws PathIOE

##########
File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java
##########
@@ -229,12 +233,45 @@ public static FileContext 
createTestFileContext(Configuration conf)
     // make this whole class not run by default
     Assume.assumeTrue("No test filesystem in " + TEST_FS_S3A_NAME,
         liveTest);
+    // Skip if S3Guard and S3-CSE are enabled.
+    skipIfS3GuardAndS3CSEEnabled(conf);
     // patch in S3Guard options
     maybeEnableS3Guard(conf);
     FileContext fc = FileContext.getFileContext(testURI, conf);
     return fc;
   }
 
+  /**
+   * Skip if S3Guard and S3CSE are enabled together.
+   *
+   * @param conf Configuration used to verify if S3Guard and S3CSE are enabled.
+   */
+  private static void skipIfS3GuardAndS3CSEEnabled(Configuration conf) {
+    String encryptionMethod =
+        conf.getTrimmed(SERVER_SIDE_ENCRYPTION_ALGORITHM, "");
+    String metaStore = conf.getTrimmed(S3_METADATA_STORE_IMPL, "");
+    if (encryptionMethod.equals(S3AEncryptionMethods.CSE_KMS.getMethod()) &&
+        !metaStore.equals(S3GUARD_METASTORE_NULL)) {
+      skip("Skipped if CSE is enabled with S3Guard.");
+    }
+  }
+
+  /**
+   * Either skip if PathIOE occurred due to S3CSE and S3Guard
+   * incompatibility or throw the PathIOE.
+   *
+   * @param ioe PathIOE being parsed.
+   * @throws PathIOException Throw the PathIOE if it doesn't relate to S3CSE
+   *                         and S3Guard incompatibility.
+   */
+  public static void maybeSkipIfS3GuardAndS3CSEIOE(PathIOException ioe)
+      throws PathIOException {
+    if (ioe.toString().contains(InternalConstants.CSE_S3GUARD_INCOMPATIBLE)) {
+      skip("Skipped if CSE is enabled with S3Guard.");

Review comment:
       nit: Skipping since.

##########
File path: 
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractSeek.java
##########
@@ -143,7 +143,8 @@ protected AbstractFSContract createContract(Configuration 
conf) {
   public void teardown() throws Exception {
     super.teardown();
     S3AFileSystem fs = getFileSystem();
-    if (fs.getConf().getBoolean(FS_S3A_IMPL_DISABLE_CACHE, false)) {
+    if (fs != null && fs.getConf().getBoolean(FS_S3A_IMPL_DISABLE_CACHE,

Review comment:
       Are these null check required?




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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 633503)
    Time Spent: 0.5h  (was: 20m)

> S3A Tests to skip if S3Guard and S3-CSE are enabled.
> ----------------------------------------------------
>
>                 Key: HADOOP-17823
>                 URL: https://issues.apache.org/jira/browse/HADOOP-17823
>             Project: Hadoop Common
>          Issue Type: Bug
>            Reporter: Mehakmeet Singh
>            Assignee: Mehakmeet Singh
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> Skip S3A tests when S3Guard and S3-CSE are enabled since it causes PathIOE 
> otherwise.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

Reply via email to