[ 
https://issues.apache.org/jira/browse/HADOOP-18980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17830560#comment-17830560
 ] 

ASF GitHub Bot commented on HADOOP-18980:
-----------------------------------------

steveloughran commented on code in PR #6546:
URL: https://github.com/apache/hadoop/pull/6546#discussion_r1537853951


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringUtils.java:
##########
@@ -79,6 +79,16 @@ public class StringUtils {
   public static final Pattern ENV_VAR_PATTERN = Shell.WINDOWS ?
     WIN_ENV_VAR_PATTERN : SHELL_ENV_VAR_PATTERN;
 
+  /**
+   * {@link #getTrimmedStringCollectionSplitByEquals(String)} throws
+   * {@link IllegalArgumentException} with error message starting with this 
string
+   * if the argument provided is not valid representation of non-empty 
key-value
+   * pairs.

Review Comment:
   tag as @VisibleForTesting, add a {@value} element to the javadocs



##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java:
##########
@@ -790,6 +790,84 @@ public void testStringCollectionSplitByEquals() {
         .containsEntry("element.xyz.key5", "element.abc.val5")
         .containsEntry("element.xyz.key6", "element.abc.val6")
         .containsEntry("element.xyz.key7", "element.abc.val7");
+
+    configuration.set("custom_key",
+        "element.first.key1 = element.first.val2 ,element.first.key1 
=element.first.val1");
+    splitMap =
+        S3AUtils.getTrimmedStringCollectionSplitByEquals(
+            configuration, "custom_key");
+    Assertions
+        .assertThat(splitMap)
+        .describedAs("Map of key value pairs split by equals(=) and comma(,)")
+        .hasSize(1)
+        .containsEntry("element.first.key1", "element.first.val1");
+
+    configuration.set("custom_key",
+        ",,, , ,, ,element.first.key1 = element.first.val2 ,"
+            + "element.first.key1 = element.first.val1 , ,,, ,");
+    splitMap = S3AUtils.getTrimmedStringCollectionSplitByEquals(
+        configuration, "custom_key");
+    Assertions
+        .assertThat(splitMap)
+        .describedAs("Map of key value pairs split by equals(=) and comma(,)")
+        .hasSize(1)
+        .containsEntry("element.first.key1", "element.first.val1");
+
+    configuration.set("custom_key", ",, , ,      ,, ,");
+    splitMap = S3AUtils.getTrimmedStringCollectionSplitByEquals(
+        configuration, "custom_key");
+    Assertions
+        .assertThat(splitMap)
+        .describedAs("Map of key value pairs split by equals(=) and comma(,)")
+        .hasSize(0);
+  }
+
+  /**
+   * Tests for the string utility that will be used by S3A credentials 
provider.
+   */
+  @Test
+  public void testStringCollectionSplitByEqualsFailure() throws Exception {

Review Comment:
   this would be better off with a helper method "expectInvalidArgument(value)" 
and 5 separate test methods. less code, more isolation.





> S3A credential provider remapping: make extensible
> --------------------------------------------------
>
>                 Key: HADOOP-18980
>                 URL: https://issues.apache.org/jira/browse/HADOOP-18980
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/s3
>    Affects Versions: 3.4.0
>            Reporter: Steve Loughran
>            Assignee: Viraj Jasani
>            Priority: Minor
>              Labels: pull-request-available
>             Fix For: 3.4.0, 3.5.0, 3.4.1
>
>
> s3afs will now remap the common com.amazonaws credential providers to 
> equivalents in the v2 sdk or in hadoop-aws
> We could do the same for third party credential providers by taking a 
> key=value list in a configuration property and adding to the map. 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to