Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: fs/s3
    • Labels:
      None

      Description

      S3a now supports different regions, by way of declaring the endpoint —but you can't do things like read in one region, write back in another (e.g. a distcp backup), because only one region can be specified in a configuration.

      If s3a supported region declaration in the URL, e.g. s3a://b1.frankfurt s3a://b2.seol , then this would be possible.

      Swift does this with a full filesystem binding/config: endpoints, username, etc, in the XML file. Would we need to do that much? It'd be simpler initially to use a domain suffix of a URL to set the region of a bucket from the domain and have the aws library sort the details out itself, maybe with some config options for working with non-AWS infra

      1. HADOOP-13336-006.patch
        25 kB
        Steve Loughran
      2. HADOOP-13336-007.patch
        26 kB
        Steve Loughran
      3. HADOOP-13336-010.patch
        33 kB
        Steve Loughran
      4. HADOOP-13336-011.patch
        33 kB
        Steve Loughran
      5. HADOOP-13336-HADOOP-13345-001.patch
        76 kB
        Steve Loughran
      6. HADOOP-13336-HADOOP-13345-002.patch
        25 kB
        Steve Loughran
      7. HADOOP-13336-HADOOP-13345-003.patch
        34 kB
        Steve Loughran
      8. HADOOP-13336-HADOOP-13345-004.patch
        34 kB
        Steve Loughran
      9. HADOOP-13336-HADOOP-13345-005.patch
        34 kB
        Steve Loughran
      10. HADOOP-13336-HADOOP-13345-006.patch
        35 kB
        Steve Loughran
      11. HADOOP-13336-HADOOP-13345-008.patch
        42 kB
        Steve Loughran
      12. HADOOP-13336-HADOOP-13345-009.patch
        42 kB
        Steve Loughran
      13. HADOOP-13336-HADOOP-13345-010.patch
        42 kB
        Steve Loughran

        Issue Links

          Activity

          Hide
          stevel@apache.org Steve Loughran added a comment -

          This is now in Hadoop 2.8+, meaning everyone will get the ability to declare different endpoints, auth and fadvice options for different buckets.

          thank you to everyone who reviewed, from core design to the security auditing.

          For reference, this was the merge strategy. I don't know if it was ideal, and it was slow/careful going, but it worked (AFAIK)

          1. apply trunk branch
          2. cherry pick to branch-2; test/verify
          3. cherry pick to branch 2.8; test verify (and when problem found, fix and build new patch.
          4. cherry pick to branch 2.8.0
          5. fork off Hadoop-13345 branch, apply trunk 012 patch, and generate diff between that and the branch with the full HADOOP-13336-HADOOP-13445-013 patch
          6. merge trunk into HADOOP-13345 branch
          7. apply the diff previously generated
          8. commit and push

          Yes, this did take >1 attempt

          Show
          stevel@apache.org Steve Loughran added a comment - This is now in Hadoop 2.8+, meaning everyone will get the ability to declare different endpoints, auth and fadvice options for different buckets. thank you to everyone who reviewed, from core design to the security auditing. For reference, this was the merge strategy. I don't know if it was ideal, and it was slow/careful going, but it worked (AFAIK) apply trunk branch cherry pick to branch-2; test/verify cherry pick to branch 2.8; test verify (and when problem found, fix and build new patch. cherry pick to branch 2.8.0 fork off Hadoop-13345 branch, apply trunk 012 patch, and generate diff between that and the branch with the full HADOOP-13336 - HADOOP-13445 -013 patch merge trunk into HADOOP-13345 branch apply the diff previously generated commit and push Yes, this did take >1 attempt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #11107 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11107/)
          HADOOP-13336 S3A to support per-bucket configuration. Contributed by (stevel: rev e648b6e1382336af69434dfbf9161bced3caa244)

          • (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java
          • (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
          • (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestConstants.java
          • (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml
          • (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java
          • (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java
          • (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/S3AScaleTestBase.java
          • (edit) hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md
          • (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAWSCredentialsProvider.java
          • (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/ITestS3AInputStreamPerformance.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #11107 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11107/ ) HADOOP-13336 S3A to support per-bucket configuration. Contributed by (stevel: rev e648b6e1382336af69434dfbf9161bced3caa244) (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestConstants.java (edit) hadoop-common-project/hadoop-common/src/main/resources/core-default.xml (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java (edit) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/S3AScaleTestBase.java (edit) hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAWSCredentialsProvider.java (edit) hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/ITestS3AInputStreamPerformance.java
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
          0 mvndep 0m 16s Maven dependency ordering for branch
          +1 mvninstall 12m 42s trunk passed
          +1 compile 9m 43s trunk passed
          +1 checkstyle 1m 32s trunk passed
          +1 mvnsite 1m 28s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          +1 findbugs 2m 3s trunk passed
          +1 javadoc 1m 10s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 0s the patch passed
          +1 compile 9m 23s the patch passed
          +1 javac 9m 23s the patch passed
          +1 checkstyle 1m 35s root: The patch generated 0 new + 30 unchanged - 1 fixed = 30 total (was 31)
          +1 mvnsite 1m 33s the patch passed
          +1 mvneclipse 0m 45s the patch passed
          -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 2m 25s the patch passed
          +1 javadoc 1m 11s the patch passed
          +1 unit 8m 14s hadoop-common in the patch passed.
          +1 unit 0m 34s hadoop-aws in the patch passed.
          +1 asflicense 0m 37s The patch does not generate ASF License warnings.
          81m 24s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846841/HADOOP-13336-011.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
          uname Linux c39f99865c8d 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / a5ec1e3
          Default Java 1.8.0_111
          findbugs v3.0.0
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11419/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11419/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11419/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files. 0 mvndep 0m 16s Maven dependency ordering for branch +1 mvninstall 12m 42s trunk passed +1 compile 9m 43s trunk passed +1 checkstyle 1m 32s trunk passed +1 mvnsite 1m 28s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 2m 3s trunk passed +1 javadoc 1m 10s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 0s the patch passed +1 compile 9m 23s the patch passed +1 javac 9m 23s the patch passed +1 checkstyle 1m 35s root: The patch generated 0 new + 30 unchanged - 1 fixed = 30 total (was 31) +1 mvnsite 1m 33s the patch passed +1 mvneclipse 0m 45s the patch passed -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 2m 25s the patch passed +1 javadoc 1m 11s the patch passed +1 unit 8m 14s hadoop-common in the patch passed. +1 unit 0m 34s hadoop-aws in the patch passed. +1 asflicense 0m 37s The patch does not generate ASF License warnings. 81m 24s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846841/HADOOP-13336-011.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux c39f99865c8d 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a5ec1e3 Default Java 1.8.0_111 findbugs v3.0.0 whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11419/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11419/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11419/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Turns out that the patch doesn't work in branch-2.8, as HADOOP_SECURITY_CREDENTIAL_PROVIDER_PATH isn't in CommonConfigurationKeysPublic.

          Patch 011 inlines the property to S3AUtils, makes package private for testing. Although it's only needed for Branch-2.8, I'm going to apply it everywhere for consistency

          Show
          stevel@apache.org Steve Loughran added a comment - Turns out that the patch doesn't work in branch-2.8, as HADOOP_SECURITY_CREDENTIAL_PROVIDER_PATH isn't in CommonConfigurationKeysPublic. Patch 011 inlines the property to S3AUtils, makes package private for testing. Although it's only needed for Branch-2.8, I'm going to apply it everywhere for consistency
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
          0 mvndep 0m 14s Maven dependency ordering for branch
          +1 mvninstall 12m 23s trunk passed
          +1 compile 9m 32s trunk passed
          +1 checkstyle 1m 32s trunk passed
          +1 mvnsite 1m 26s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          +1 findbugs 1m 58s trunk passed
          +1 javadoc 1m 12s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 0m 58s the patch passed
          +1 compile 9m 14s the patch passed
          +1 javac 9m 14s the patch passed
          -0 checkstyle 1m 35s root: The patch generated 2 new + 30 unchanged - 1 fixed = 32 total (was 31)
          +1 mvnsite 1m 30s the patch passed
          +1 mvneclipse 0m 46s the patch passed
          -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 2m 42s the patch passed
          +1 javadoc 1m 16s the patch passed
          +1 unit 9m 17s hadoop-common in the patch passed.
          +1 unit 0m 34s hadoop-aws in the patch passed.
          +1 asflicense 0m 37s The patch does not generate ASF License warnings.
          81m 58s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846650/HADOOP-13336-010.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
          uname Linux 7f5e8229b08f 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / e692316
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11412/artifact/patchprocess/diff-checkstyle-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11412/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11412/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11412/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files. 0 mvndep 0m 14s Maven dependency ordering for branch +1 mvninstall 12m 23s trunk passed +1 compile 9m 32s trunk passed +1 checkstyle 1m 32s trunk passed +1 mvnsite 1m 26s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 1m 58s trunk passed +1 javadoc 1m 12s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 0m 58s the patch passed +1 compile 9m 14s the patch passed +1 javac 9m 14s the patch passed -0 checkstyle 1m 35s root: The patch generated 2 new + 30 unchanged - 1 fixed = 32 total (was 31) +1 mvnsite 1m 30s the patch passed +1 mvneclipse 0m 46s the patch passed -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 2m 42s the patch passed +1 javadoc 1m 16s the patch passed +1 unit 9m 17s hadoop-common in the patch passed. +1 unit 0m 34s hadoop-aws in the patch passed. +1 asflicense 0m 37s The patch does not generate ASF License warnings. 81m 58s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846650/HADOOP-13336-010.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux 7f5e8229b08f 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / e692316 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11412/artifact/patchprocess/diff-checkstyle-root.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11412/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11412/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11412/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          the complaint about an unused import is wrong: its referenced in the javadocs

          It happens to me a few of times. I used the full path (package) of a class in javadoc and removed the import statement to resolve this. Not ideal anyway.

          Show
          liuml07 Mingliang Liu added a comment - the complaint about an unused import is wrong: its referenced in the javadocs It happens to me a few of times. I used the full path (package) of a class in javadoc and removed the import statement to resolve this. Not ideal anyway.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          and trunk version of the same patch

          Show
          stevel@apache.org Steve Loughran added a comment - and trunk version of the same patch
          Hide
          stevel@apache.org Steve Loughran added a comment -

          checkstyle complained about some of the style aspects of the last patch. here: fixed. No source code changes other than indentation and line wrapping, so I'm assuming the existing +1 holds. This is just me being consistent.

          the complaint about an unused import is wrong: its referenced in the javadocs

          Show
          stevel@apache.org Steve Loughran added a comment - checkstyle complained about some of the style aspects of the last patch. here: fixed. No source code changes other than indentation and line wrapping, so I'm assuming the existing +1 holds. This is just me being consistent. the complaint about an unused import is wrong: its referenced in the javadocs
          Hide
          lmccay Larry McCay added a comment -

          That sounds like right to me, Steve Loughran.

          Show
          lmccay Larry McCay added a comment - That sounds like right to me, Steve Loughran .
          Hide
          stevel@apache.org Steve Loughran added a comment -

          OK, thanks for the vote. Can I be the one to (carefully) commit this. I plan to

          1. commit to trunk & branch-2
          2. merge trunk into the s3guard branch
          3. apply the bits of the the patch for the s3guard branch which aren't in the trunk branch

          That way, I can get into the mainline branches a feature that is needed for many things (primarily cross v4 auth site and to use different credentials without embedding secrets in URIs), without creating merge pain downstream.

          Show
          stevel@apache.org Steve Loughran added a comment - OK, thanks for the vote. Can I be the one to (carefully) commit this. I plan to commit to trunk & branch-2 merge trunk into the s3guard branch apply the bits of the the patch for the s3guard branch which aren't in the trunk branch That way, I can get into the mainline branches a feature that is needed for many things (primarily cross v4 auth site and to use different credentials without embedding secrets in URIs), without creating merge pain downstream.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 10 new or modified test files.
          0 mvndep 1m 53s Maven dependency ordering for branch
          +1 mvninstall 7m 20s HADOOP-13345 passed
          +1 compile 9m 36s HADOOP-13345 passed
          +1 checkstyle 1m 33s HADOOP-13345 passed
          +1 mvnsite 1m 36s HADOOP-13345 passed
          +1 mvneclipse 0m 41s HADOOP-13345 passed
          +1 findbugs 2m 7s HADOOP-13345 passed
          +1 javadoc 1m 14s HADOOP-13345 passed
          0 mvndep 0m 18s Maven dependency ordering for patch
          +1 mvninstall 1m 2s the patch passed
          +1 compile 9m 41s the patch passed
          +1 javac 9m 41s the patch passed
          -0 checkstyle 1m 40s root: The patch generated 7 new + 34 unchanged - 1 fixed = 41 total (was 35)
          +1 mvnsite 1m 39s the patch passed
          +1 mvneclipse 0m 45s the patch passed
          -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 xml 0m 2s The patch has no ill-formed XML file.
          +1 findbugs 2m 25s the patch passed
          +1 javadoc 1m 17s the patch passed
          +1 unit 8m 41s hadoop-common in the patch passed.
          +1 unit 0m 47s hadoop-aws in the patch passed.
          +1 asflicense 0m 38s The patch does not generate ASF License warnings.
          78m 50s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846558/HADOOP-13336-HADOOP-13345-009.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
          uname Linux 640e96812949 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision HADOOP-13345 / a5cc315
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11408/artifact/patchprocess/diff-checkstyle-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11408/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11408/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11408/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 10 new or modified test files. 0 mvndep 1m 53s Maven dependency ordering for branch +1 mvninstall 7m 20s HADOOP-13345 passed +1 compile 9m 36s HADOOP-13345 passed +1 checkstyle 1m 33s HADOOP-13345 passed +1 mvnsite 1m 36s HADOOP-13345 passed +1 mvneclipse 0m 41s HADOOP-13345 passed +1 findbugs 2m 7s HADOOP-13345 passed +1 javadoc 1m 14s HADOOP-13345 passed 0 mvndep 0m 18s Maven dependency ordering for patch +1 mvninstall 1m 2s the patch passed +1 compile 9m 41s the patch passed +1 javac 9m 41s the patch passed -0 checkstyle 1m 40s root: The patch generated 7 new + 34 unchanged - 1 fixed = 41 total (was 35) +1 mvnsite 1m 39s the patch passed +1 mvneclipse 0m 45s the patch passed -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 xml 0m 2s The patch has no ill-formed XML file. +1 findbugs 2m 25s the patch passed +1 javadoc 1m 17s the patch passed +1 unit 8m 41s hadoop-common in the patch passed. +1 unit 0m 47s hadoop-aws in the patch passed. +1 asflicense 0m 38s The patch does not generate ASF License warnings. 78m 50s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846558/HADOOP-13336-HADOOP-13345-009.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux 640e96812949 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision HADOOP-13345 / a5cc315 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11408/artifact/patchprocess/diff-checkstyle-root.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11408/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11408/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11408/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          lmccay Larry McCay added a comment -

          LGTM

          +1

          Show
          lmccay Larry McCay added a comment - LGTM +1
          Hide
          stevel@apache.org Steve Loughran added a comment -

          tested: s3a ireland.

          Show
          stevel@apache.org Steve Loughran added a comment - tested: s3a ireland.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          patch 009: don't log value of updated property for security reasons; just the name of the patched value

          Show
          stevel@apache.org Steve Loughran added a comment - patch 009: don't log value of updated property for security reasons; just the name of the patched value
          Hide
          stevel@apache.org Steve Loughran added a comment -

          these are things like endpoint, etc. But yes, if there are security things in there too.

          I'll just log the "updated" fact, rather than the value -If I tried to be clever about which properties shouldn't be logged, it'd only be brittle and probably still insecure

          Show
          stevel@apache.org Steve Loughran added a comment - these are things like endpoint, etc. But yes, if there are security things in there too. I'll just log the "updated" fact, rather than the value -If I tried to be clever about which properties shouldn't be logged, it'd only be brittle and probably still insecure
          Hide
          lmccay Larry McCay added a comment -

          Steve Loughran - Ahhh - I missed the constant names difference.
          Eyes are getting tired (old). I thought that I could see some difference but I couldn't find it.

          The patchCredentialProvider approach looks good.

          I am still curious whether the following will end up logging something sensitive - given that this is all about providing separate credentials per bucket:

          +        LOG.debug("Setting {} to value of {} : {} (was {})",
          +            generic, key, value, source.get(generic));
          

          Am I missing what the "value" of these properties actually are?

          Show
          lmccay Larry McCay added a comment - Steve Loughran - Ahhh - I missed the constant names difference. Eyes are getting tired (old). I thought that I could see some difference but I couldn't find it. The patchCredentialProvider approach looks good. I am still curious whether the following will end up logging something sensitive - given that this is all about providing separate credentials per bucket: + LOG.debug("Setting {} to value of {} : {} (was {})", + generic, key, value, source.get(generic)); Am I missing what the "value" of these properties actually are?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 10 new or modified test files.
          0 mvndep 1m 53s Maven dependency ordering for branch
          +1 mvninstall 8m 20s HADOOP-13345 passed
          +1 compile 10m 31s HADOOP-13345 passed
          +1 checkstyle 1m 34s HADOOP-13345 passed
          +1 mvnsite 1m 35s HADOOP-13345 passed
          +1 mvneclipse 0m 40s HADOOP-13345 passed
          +1 findbugs 2m 3s HADOOP-13345 passed
          +1 javadoc 1m 11s HADOOP-13345 passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 0s the patch passed
          +1 compile 9m 15s the patch passed
          +1 javac 9m 15s the patch passed
          -0 checkstyle 1m 35s root: The patch generated 7 new + 34 unchanged - 1 fixed = 41 total (was 35)
          +1 mvnsite 1m 37s the patch passed
          +1 mvneclipse 0m 46s the patch passed
          -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 2m 22s the patch passed
          +1 javadoc 1m 17s the patch passed
          +1 unit 8m 5s hadoop-common in the patch passed.
          +1 unit 0m 48s hadoop-aws in the patch passed.
          +1 asflicense 0m 38s The patch does not generate ASF License warnings.
          79m 32s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846406/HADOOP-13336-HADOOP-13345-008.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
          uname Linux 5e1c74172b14 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision HADOOP-13345 / e3f2002
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11401/artifact/patchprocess/diff-checkstyle-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11401/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11401/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11401/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 10 new or modified test files. 0 mvndep 1m 53s Maven dependency ordering for branch +1 mvninstall 8m 20s HADOOP-13345 passed +1 compile 10m 31s HADOOP-13345 passed +1 checkstyle 1m 34s HADOOP-13345 passed +1 mvnsite 1m 35s HADOOP-13345 passed +1 mvneclipse 0m 40s HADOOP-13345 passed +1 findbugs 2m 3s HADOOP-13345 passed +1 javadoc 1m 11s HADOOP-13345 passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 0s the patch passed +1 compile 9m 15s the patch passed +1 javac 9m 15s the patch passed -0 checkstyle 1m 35s root: The patch generated 7 new + 34 unchanged - 1 fixed = 41 total (was 35) +1 mvnsite 1m 37s the patch passed +1 mvneclipse 0m 46s the patch passed -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 2m 22s the patch passed +1 javadoc 1m 17s the patch passed +1 unit 8m 5s hadoop-common in the patch passed. +1 unit 0m 48s hadoop-aws in the patch passed. +1 asflicense 0m 38s The patch does not generate ASF License warnings. 79m 32s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846406/HADOOP-13336-HADOOP-13345-008.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux 5e1c74172b14 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision HADOOP-13345 / e3f2002 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11401/artifact/patchprocess/diff-checkstyle-root.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11401/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11401/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11401/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          tested, s3a ireland

          Show
          stevel@apache.org Steve Loughran added a comment - tested, s3a ireland
          Hide
          stevel@apache.org Steve Loughran added a comment -

          HADOOP-13336 patch 008: permit bucket customisation of hadoop credential providers, by adding an fs.s3a. property for declaring an extra provider path.

          This means the same property names inside the files are used, you just give a different file for each endpoint.

          Show
          stevel@apache.org Steve Loughran added a comment - HADOOP-13336 patch 008: permit bucket customisation of hadoop credential providers, by adding an fs.s3a. property for declaring an extra provider path. This means the same property names inside the files are used, you just give a different file for each endpoint.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Larrrt there's a scan for a forbidden prefix (currently bucket.") and then actual unmodifiable value. I actually think I could fix them to just the simple checks, and not-overengineer this.

          w.r.t Configuration.getPassword(), I see the problem you are alluding to. Even though we are migrating fs.s3a.bucket.* to fs.s3a.*, that does nothing to the credential providers, as they have hard-coded keys in their key:value mappings; this isn't changing anything.

          hmmm.

          Would it be possible for us to update the "hadoop.security.credential.provider.path" at the same time, that is add a special property to s3a, say fs.s3a.security.credential.provider.path. All the contents of that property would be prepanded to that of the base one. You'd then need to specify different providers for the different endpoints. By prepending the values, we can ensure that properties stated in a bucket will take priority over any in the default provider path.

          We'd need to document this, especially how it's likely that once there's a secret in a JCEKS file, then you must overrride those secrets with new files: you can't move back to a password from a credentials file: you can't downgrade security.

          Would that work? If so, I can include that in this patch as it's related to the per-bucket config, isn't it?

          Show
          stevel@apache.org Steve Loughran added a comment - Larrrt there's a scan for a forbidden prefix (currently bucket.") and then actual unmodifiable value. I actually think I could fix them to just the simple checks, and not-overengineer this. w.r.t Configuration.getPassword(), I see the problem you are alluding to. Even though we are migrating fs.s3a.bucket.* to fs.s3a.*, that does nothing to the credential providers, as they have hard-coded keys in their key:value mappings; this isn't changing anything. hmmm. Would it be possible for us to update the "hadoop.security.credential.provider.path" at the same time, that is add a special property to s3a, say fs.s3a.security.credential.provider.path . All the contents of that property would be prepanded to that of the base one. You'd then need to specify different providers for the different endpoints. By prepending the values, we can ensure that properties stated in a bucket will take priority over any in the default provider path. We'd need to document this, especially how it's likely that once there's a secret in a JCEKS file, then you must overrride those secrets with new files: you can't move back to a password from a credentials file: you can't downgrade security. Would that work? If so, I can include that in this patch as it's related to the per-bucket config, isn't it?
          Hide
          cnauroth Chris Nauroth added a comment -

          I'm in favor of the approach, and there are already plenty of reviewers commenting, so I'll focus my code review energies elsewhere. Thanks, Steve!

          Show
          cnauroth Chris Nauroth added a comment - I'm in favor of the approach, and there are already plenty of reviewers commenting, so I'll focus my code review energies elsewhere. Thanks, Steve!
          Hide
          lmccay Larry McCay added a comment -

          Hi Steve Loughran -

          This looks like a good approach - couple comments:

          • In propagateBucketOptions it seems that there may be a redundant loop:
          +      for (String unmodifiable : UNMODIFIABLE_PREFIX) {
          +        if (stripped.startsWith(unmodifiable)) {
          +          //tell user off
          +          LOG.debug("Ignoring bucket option {}", key);
          +          skip = true;
          +        }
          +      }
          +      for (String unmodifiable : UNMODIFIABLE_VALUE) {
          +        if (stripped.equals(unmodifiable)) {
          +          //tell user off
          +          LOG.debug("Ignoring bucket option {}", key);
          +          skip = true;
          +        }
          +      }
          
          • would it also be better to have the log message mention why it is being ignored?
          • would the following code in the same method result in passwords being logged by any chance:
            +      // propagate the value, building a new origin field.
            +      // to track overwrites, the generic key is overwritten even if
            +      // already matches the new one.
            +      if (!skip) {
            +        final String generic = FS_S3A_PREFIX + stripped;
            +        LOG.debug("Setting {} to value of {} : {} (was {})",
            +            generic, key, value, source.get(generic));
            +        dest.set(generic, value, key);
            +      }
            
          • I don't see any evidence that this will work with credential provider API through the use of Configuration.getPassword. Am I missing some other nuance here?
          Show
          lmccay Larry McCay added a comment - Hi Steve Loughran - This looks like a good approach - couple comments: In propagateBucketOptions it seems that there may be a redundant loop: + for (String unmodifiable : UNMODIFIABLE_PREFIX) { + if (stripped.startsWith(unmodifiable)) { + //tell user off + LOG.debug("Ignoring bucket option {}", key); + skip = true; + } + } + for (String unmodifiable : UNMODIFIABLE_VALUE) { + if (stripped.equals(unmodifiable)) { + //tell user off + LOG.debug("Ignoring bucket option {}", key); + skip = true; + } + } would it also be better to have the log message mention why it is being ignored? would the following code in the same method result in passwords being logged by any chance: + // propagate the value, building a new origin field. + // to track overwrites, the generic key is overwritten even if + // already matches the new one. + if (!skip) { + final String generic = FS_S3A_PREFIX + stripped; + LOG.debug("Setting {} to value of {} : {} (was {})", + generic, key, value, source.get(generic)); + dest.set(generic, value, key); + } I don't see any evidence that this will work with credential provider API through the use of Configuration.getPassword. Am I missing some other nuance here?
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
          +1 mvninstall 13m 2s trunk passed
          +1 compile 0m 19s trunk passed
          +1 checkstyle 0m 14s trunk passed
          +1 mvnsite 0m 20s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 27s trunk passed
          +1 javadoc 0m 13s trunk passed
          +1 mvninstall 0m 16s the patch passed
          +1 compile 0m 16s the patch passed
          +1 javac 0m 16s the patch passed
          +1 checkstyle 0m 11s hadoop-tools/hadoop-aws: The patch generated 0 new + 30 unchanged - 1 fixed = 30 total (was 31)
          +1 mvnsite 0m 19s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 31s the patch passed
          +1 javadoc 0m 11s the patch passed
          +1 unit 0m 20s hadoop-aws in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          18m 57s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846320/HADOOP-13336-007.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 10847234dc95 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / db490ec
          Default Java 1.8.0_111
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11398/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11398/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files. +1 mvninstall 13m 2s trunk passed +1 compile 0m 19s trunk passed +1 checkstyle 0m 14s trunk passed +1 mvnsite 0m 20s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 27s trunk passed +1 javadoc 0m 13s trunk passed +1 mvninstall 0m 16s the patch passed +1 compile 0m 16s the patch passed +1 javac 0m 16s the patch passed +1 checkstyle 0m 11s hadoop-tools/hadoop-aws: The patch generated 0 new + 30 unchanged - 1 fixed = 30 total (was 31) +1 mvnsite 0m 19s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 31s the patch passed +1 javadoc 0m 11s the patch passed +1 unit 0m 20s hadoop-aws in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 18m 57s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846320/HADOOP-13336-007.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 10847234dc95 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / db490ec Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11398/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11398/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Patch 007: latest test fixes, including the checkstyle ones

          Show
          stevel@apache.org Steve Loughran added a comment - Patch 007: latest test fixes, including the checkstyle ones
          Hide
          stevel@apache.org Steve Loughran added a comment -

          cancelling patch; trunk branch was 1 commit behind

          Show
          stevel@apache.org Steve Loughran added a comment - cancelling patch; trunk branch was 1 commit behind
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 6 new or modified test files.
          +1 mvninstall 12m 40s trunk passed
          +1 compile 0m 18s trunk passed
          +1 checkstyle 0m 13s trunk passed
          +1 mvnsite 0m 20s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          +1 findbugs 0m 27s trunk passed
          +1 javadoc 0m 14s trunk passed
          +1 mvninstall 0m 17s the patch passed
          +1 compile 0m 16s the patch passed
          +1 javac 0m 16s the patch passed
          -0 checkstyle 0m 11s hadoop-tools/hadoop-aws: The patch generated 2 new + 30 unchanged - 1 fixed = 32 total (was 31)
          +1 mvnsite 0m 17s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 31s the patch passed
          +1 javadoc 0m 11s the patch passed
          +1 unit 0m 19s hadoop-aws in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          18m 32s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846315/HADOOP-13336-006.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 21fe0e95d5e2 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / db490ec
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11397/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11397/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11397/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 6 new or modified test files. +1 mvninstall 12m 40s trunk passed +1 compile 0m 18s trunk passed +1 checkstyle 0m 13s trunk passed +1 mvnsite 0m 20s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 0m 27s trunk passed +1 javadoc 0m 14s trunk passed +1 mvninstall 0m 17s the patch passed +1 compile 0m 16s the patch passed +1 javac 0m 16s the patch passed -0 checkstyle 0m 11s hadoop-tools/hadoop-aws: The patch generated 2 new + 30 unchanged - 1 fixed = 32 total (was 31) +1 mvnsite 0m 17s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 31s the patch passed +1 javadoc 0m 11s the patch passed +1 unit 0m 19s hadoop-aws in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 18m 32s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846315/HADOOP-13336-006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 21fe0e95d5e2 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / db490ec Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11397/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11397/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11397/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Patch 006 against trunk. Main diff is in testing, plus some changes to S3AFS aren't needed/valid

          Show
          stevel@apache.org Steve Loughran added a comment - Patch 006 against trunk. Main diff is in testing, plus some changes to S3AFS aren't needed/valid
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 10 new or modified test files.
          +1 mvninstall 6m 44s HADOOP-13345 passed
          +1 compile 0m 20s HADOOP-13345 passed
          +1 checkstyle 0m 14s HADOOP-13345 passed
          +1 mvnsite 0m 25s HADOOP-13345 passed
          +1 mvneclipse 0m 16s HADOOP-13345 passed
          +1 findbugs 0m 27s HADOOP-13345 passed
          +1 javadoc 0m 14s HADOOP-13345 passed
          +1 mvninstall 0m 19s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          +1 checkstyle 0m 11s hadoop-tools/hadoop-aws: The patch generated 0 new + 34 unchanged - 1 fixed = 34 total (was 35)
          +1 mvnsite 0m 23s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 32s the patch passed
          +1 javadoc 0m 11s the patch passed
          +1 unit 0m 33s hadoop-aws in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          13m 8s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846193/HADOOP-13336-HADOOP-13345-006.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 2d680462cab9 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision HADOOP-13345 / a1b47db
          Default Java 1.8.0_111
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11391/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11391/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 10 new or modified test files. +1 mvninstall 6m 44s HADOOP-13345 passed +1 compile 0m 20s HADOOP-13345 passed +1 checkstyle 0m 14s HADOOP-13345 passed +1 mvnsite 0m 25s HADOOP-13345 passed +1 mvneclipse 0m 16s HADOOP-13345 passed +1 findbugs 0m 27s HADOOP-13345 passed +1 javadoc 0m 14s HADOOP-13345 passed +1 mvninstall 0m 19s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed +1 checkstyle 0m 11s hadoop-tools/hadoop-aws: The patch generated 0 new + 34 unchanged - 1 fixed = 34 total (was 35) +1 mvnsite 0m 23s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 32s the patch passed +1 javadoc 0m 11s the patch passed +1 unit 0m 33s hadoop-aws in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 13m 8s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846193/HADOOP-13336-HADOOP-13345-006.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2d680462cab9 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision HADOOP-13345 / a1b47db Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11391/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11391/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          patch 006.

          • fix checkstyle
          • ITestS3AConfiguration switch to inline constants for the s3guard constants. Needed to ensure the tests also build on trunk
          • Added a test which explicitly verifies properties propagate to fs.getConf(), rather than relying on tester to set up s3a endpoint which couldnt' reach the landsat image, or similar.
          Show
          stevel@apache.org Steve Loughran added a comment - patch 006. fix checkstyle ITestS3AConfiguration switch to inline constants for the s3guard constants. Needed to ensure the tests also build on trunk Added a test which explicitly verifies properties propagate to fs.getConf(), rather than relying on tester to set up s3a endpoint which couldnt' reach the landsat image, or similar.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Also happy the implementation turned out to be pretty simple.

          well, the first implementation I threw away. Never mind. Better than than shipping something complex and unmaintainable

          Show
          stevel@apache.org Steve Loughran added a comment - Also happy the implementation turned out to be pretty simple. well, the first implementation I threw away. Never mind. Better than than shipping something complex and unmaintainable
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 10 new or modified test files.
          +1 mvninstall 7m 21s HADOOP-13345 passed
          +1 compile 0m 20s HADOOP-13345 passed
          +1 checkstyle 0m 14s HADOOP-13345 passed
          +1 mvnsite 0m 28s HADOOP-13345 passed
          +1 mvneclipse 0m 16s HADOOP-13345 passed
          +1 findbugs 0m 27s HADOOP-13345 passed
          +1 javadoc 0m 15s HADOOP-13345 passed
          +1 mvninstall 0m 19s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 2 new + 34 unchanged - 1 fixed = 36 total (was 35)
          +1 mvnsite 0m 23s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          -1 whitespace 0m 1s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 findbugs 0m 32s the patch passed
          +1 javadoc 0m 11s the patch passed
          +1 unit 0m 32s hadoop-aws in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          13m 53s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846189/HADOOP-13336-HADOOP-13345-005.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ad7e098ee81c 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision HADOOP-13345 / a1b47db
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11390/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11390/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11390/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11390/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 10 new or modified test files. +1 mvninstall 7m 21s HADOOP-13345 passed +1 compile 0m 20s HADOOP-13345 passed +1 checkstyle 0m 14s HADOOP-13345 passed +1 mvnsite 0m 28s HADOOP-13345 passed +1 mvneclipse 0m 16s HADOOP-13345 passed +1 findbugs 0m 27s HADOOP-13345 passed +1 javadoc 0m 15s HADOOP-13345 passed +1 mvninstall 0m 19s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 2 new + 34 unchanged - 1 fixed = 36 total (was 35) +1 mvnsite 0m 23s the patch passed +1 mvneclipse 0m 13s the patch passed -1 whitespace 0m 1s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 0m 32s the patch passed +1 javadoc 0m 11s the patch passed +1 unit 0m 32s hadoop-aws in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 13m 53s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846189/HADOOP-13336-HADOOP-13345-005.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ad7e098ee81c 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision HADOOP-13345 / a1b47db Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11390/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11390/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11390/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11390/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Thanks for the vote. This is patch 005, which fixes checkstyle

          Show
          stevel@apache.org Steve Loughran added a comment - Thanks for the vote. This is patch 005, which fixes checkstyle
          Hide
          liuml07 Mingliang Liu added a comment -

          +1

          Nice work!

          I'll hold on commit in 3 days if Chris Nauroth, Sean Mackrory and Doug Laurence have further comments. Checkstyle warnings seem related.

          Show
          liuml07 Mingliang Liu added a comment - +1 Nice work! I'll hold on commit in 3 days if Chris Nauroth , Sean Mackrory and Doug Laurence have further comments. Checkstyle warnings seem related.
          Hide
          fabbri Aaron Fabbri added a comment -

          +1 (binding only on HADOOP-13345 feature branch).

          Excited about this feature. Also happy the implementation turned out to be pretty simple.

          Show
          fabbri Aaron Fabbri added a comment - +1 (binding only on HADOOP-13345 feature branch). Excited about this feature. Also happy the implementation turned out to be pretty simple.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 10 new or modified test files.
          +1 mvninstall 7m 46s HADOOP-13345 passed
          +1 compile 0m 20s HADOOP-13345 passed
          +1 checkstyle 0m 14s HADOOP-13345 passed
          +1 mvnsite 0m 27s HADOOP-13345 passed
          +1 mvneclipse 0m 15s HADOOP-13345 passed
          +1 findbugs 0m 28s HADOOP-13345 passed
          +1 javadoc 0m 16s HADOOP-13345 passed
          +1 mvninstall 0m 26s the patch passed
          +1 compile 0m 21s the patch passed
          +1 javac 0m 21s the patch passed
          -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 10 new + 34 unchanged - 1 fixed = 44 total (was 35)
          +1 mvnsite 0m 27s the patch passed
          +1 mvneclipse 0m 15s the patch passed
          -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 findbugs 0m 39s the patch passed
          +1 javadoc 0m 15s the patch passed
          +1 unit 0m 39s hadoop-aws in the patch passed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          14m 56s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846081/HADOOP-13336-HADOOP-13345-004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 787ba5340c4c 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision HADOOP-13345 / a1b47db
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11384/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11384/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11384/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11384/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 10 new or modified test files. +1 mvninstall 7m 46s HADOOP-13345 passed +1 compile 0m 20s HADOOP-13345 passed +1 checkstyle 0m 14s HADOOP-13345 passed +1 mvnsite 0m 27s HADOOP-13345 passed +1 mvneclipse 0m 15s HADOOP-13345 passed +1 findbugs 0m 28s HADOOP-13345 passed +1 javadoc 0m 16s HADOOP-13345 passed +1 mvninstall 0m 26s the patch passed +1 compile 0m 21s the patch passed +1 javac 0m 21s the patch passed -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 10 new + 34 unchanged - 1 fixed = 44 total (was 35) +1 mvnsite 0m 27s the patch passed +1 mvneclipse 0m 15s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 0m 39s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 0m 39s hadoop-aws in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 14m 56s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846081/HADOOP-13336-HADOOP-13345-004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 787ba5340c4c 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision HADOOP-13345 / a1b47db Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11384/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11384/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11384/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11384/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 11 new or modified test files.
          +1 mvninstall 7m 3s HADOOP-13345 passed
          +1 compile 0m 19s HADOOP-13345 passed
          +1 checkstyle 0m 14s HADOOP-13345 passed
          +1 mvnsite 0m 26s HADOOP-13345 passed
          +1 mvneclipse 0m 15s HADOOP-13345 passed
          +1 findbugs 0m 28s HADOOP-13345 passed
          +1 javadoc 0m 14s HADOOP-13345 passed
          +1 mvninstall 0m 19s the patch passed
          +1 compile 0m 18s the patch passed
          +1 javac 0m 18s the patch passed
          -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 9 new + 34 unchanged - 1 fixed = 43 total (was 35)
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 15s the patch passed
          -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 findbugs 0m 39s the patch passed
          +1 javadoc 0m 15s the patch passed
          +1 unit 0m 37s hadoop-aws in the patch passed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          13m 55s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846079/HADOOP-13336-HADOOP-13345-003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ae26dcd6d480 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision HADOOP-13345 / a1b47db
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11383/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11383/artifact/patchprocess/whitespace-eol.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11383/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11383/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 11 new or modified test files. +1 mvninstall 7m 3s HADOOP-13345 passed +1 compile 0m 19s HADOOP-13345 passed +1 checkstyle 0m 14s HADOOP-13345 passed +1 mvnsite 0m 26s HADOOP-13345 passed +1 mvneclipse 0m 15s HADOOP-13345 passed +1 findbugs 0m 28s HADOOP-13345 passed +1 javadoc 0m 14s HADOOP-13345 passed +1 mvninstall 0m 19s the patch passed +1 compile 0m 18s the patch passed +1 javac 0m 18s the patch passed -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 9 new + 34 unchanged - 1 fixed = 43 total (was 35) +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 15s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 0m 39s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 0m 37s hadoop-aws in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 13m 55s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846079/HADOOP-13336-HADOOP-13345-003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ae26dcd6d480 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision HADOOP-13345 / a1b47db Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11383/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11383/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11383/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11383/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          patch 004; address Aaron's comments

          Show
          stevel@apache.org Steve Loughran added a comment - patch 004; address Aaron's comments
          Hide
          stevel@apache.org Steve Loughran added a comment - - edited

          "fs.s3a.impl". is the only property you can't stamp on. I'll extend the test to make sure that the metadatatore one survivces

          value==null.

          I don't think so either, but some of the underlying code hinted at it. I'll cut it, as it shouldn't happen.
          thanks for the feedback; I'll do an iteration on it.

          regarding the log4.properties, I had left that out the stuff I was checking in, but had got my git diff wrong. Sorry.

          Show
          stevel@apache.org Steve Loughran added a comment - - edited "fs.s3a.impl". is the only property you can't stamp on. I'll extend the test to make sure that the metadatatore one survivces value==null . I don't think so either, but some of the underlying code hinted at it. I'll cut it, as it shouldn't happen. thanks for the feedback; I'll do an iteration on it. regarding the log4.properties, I had left that out the stuff I was checking in, but had got my git diff wrong. Sorry.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          patch 003. Fix javadoc. S3AFS declares whether or not it has a metadata store; use that value in the FS and tests

          Show
          stevel@apache.org Steve Loughran added a comment - patch 003. Fix javadoc. S3AFS declares whether or not it has a metadata store; use that value in the FS and tests
          Hide
          fabbri Aaron Fabbri added a comment -

          +1 (non-binding) once you remove the change of debug log level at end of patch.

          I like this approach. I had thought about this a bit before you started work and was thinking of having a "flatten" (a.k.a. propagate) function as you do here. Seems more efficient at runtime and easier to debug. I like the use of the Configuration source feature too.

          It looks like your omission of <prefix>.impl keys still works with fs.s3a.metadatastore.impl, since "impl".equals("metadatastore.impl") == false. (You compare the whole key, not just the suffix.) This is good because we'd like to be able to enable/disable s3guard on a per-bucket basis.

          Other patch comments:

          +  public static Configuration propagateBucketOptions(Configuration source,
          +      String bucket) {
          +
          <snip>
          +    for (Map.Entry<String, String> entry : source) {
          +      final String key = entry.getKey();
          +      // get the (unexpanded) value.
          +      final String value = entry.getValue();
          +      if (!key.startsWith(bucketPrefix)
          +          || bucketPrefix.equals(key)
          +          || value == null) {
          +        continue;
          +      }
          

          Was curious about the value == null part.. Does that ever happen? Anyhow, seems safe to include the check.

          Minor nit in the docs:

          +Different S3 buckets can be accessed with S3A client configurations.
          

          Should this read "can be accessed with different S3A client configurations"?

          --- a/hadoop-tools/hadoop-aws/src/test/resources/log4j.properties
          +++ b/hadoop-tools/hadoop-aws/src/test/resources/log4j.properties
          @@ -21,7 +21,7 @@ log4j.logger.org.apache.hadoop.util.NativeCodeLoader=ERROR
            
           # for debugging low level S3a operations, uncomment these lines
           # Log all S3A classes
          -#log4j.logger.org.apache.hadoop.fs.s3a=DEBUG
          +log4j.logger.org.apache.hadoop.fs.s3a=DEBUG
            
           # Log S3Guard classes
           #log4j.logger.org.apache.hadoop.fs.s3a.s3guard=DEBUG
          

          Was this intentional?

          Show
          fabbri Aaron Fabbri added a comment - +1 (non-binding) once you remove the change of debug log level at end of patch. I like this approach. I had thought about this a bit before you started work and was thinking of having a "flatten" (a.k.a. propagate) function as you do here. Seems more efficient at runtime and easier to debug. I like the use of the Configuration source feature too. It looks like your omission of <prefix>.impl keys still works with fs.s3a.metadatastore.impl, since "impl".equals("metadatastore.impl") == false . (You compare the whole key, not just the suffix.) This is good because we'd like to be able to enable/disable s3guard on a per-bucket basis. Other patch comments: + public static Configuration propagateBucketOptions(Configuration source, + String bucket) { + <snip> + for (Map.Entry< String , String > entry : source) { + final String key = entry.getKey(); + // get the (unexpanded) value. + final String value = entry.getValue(); + if (!key.startsWith(bucketPrefix) + || bucketPrefix.equals(key) + || value == null ) { + continue ; + } Was curious about the value == null part.. Does that ever happen? Anyhow, seems safe to include the check. Minor nit in the docs: +Different S3 buckets can be accessed with S3A client configurations. Should this read "can be accessed with different S3A client configurations"? --- a/hadoop-tools/hadoop-aws/src/test/resources/log4j.properties +++ b/hadoop-tools/hadoop-aws/src/test/resources/log4j.properties @@ -21,7 +21,7 @@ log4j.logger.org.apache.hadoop.util.NativeCodeLoader=ERROR # for debugging low level S3a operations, uncomment these lines # Log all S3A classes -#log4j.logger.org.apache.hadoop.fs.s3a=DEBUG +log4j.logger.org.apache.hadoop.fs.s3a=DEBUG # Log S3Guard classes #log4j.logger.org.apache.hadoop.fs.s3a.s3guard=DEBUG Was this intentional?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          cancel for javadoc.

          One other thing I'm going to change: cut isNullMetadataStoreConfigured(Configuration conf). You need to know the bucket name here. Best to ask the FS itself whether its there.

          Show
          stevel@apache.org Steve Loughran added a comment - cancel for javadoc. One other thing I'm going to change: cut isNullMetadataStoreConfigured(Configuration conf) . You need to know the bucket name here. Best to ask the FS itself whether its there.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 7 new or modified test files.
          +1 mvninstall 6m 49s HADOOP-13345 passed
          +1 compile 0m 20s HADOOP-13345 passed
          +1 checkstyle 0m 14s HADOOP-13345 passed
          +1 mvnsite 0m 26s HADOOP-13345 passed
          +1 mvneclipse 0m 16s HADOOP-13345 passed
          +1 findbugs 0m 28s HADOOP-13345 passed
          +1 javadoc 0m 15s HADOOP-13345 passed
          +1 mvninstall 0m 19s the patch passed
          +1 compile 0m 17s the patch passed
          +1 javac 0m 17s the patch passed
          -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 7 new + 33 unchanged - 0 fixed = 40 total (was 33)
          +1 mvnsite 0m 23s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 findbugs 0m 33s the patch passed
          -1 javadoc 0m 11s hadoop-aws in the patch failed.
          +1 unit 0m 33s hadoop-aws in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          13m 14s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846061/HADOOP-13336-HADOOP-13345-002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 785d1ba1db19 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision HADOOP-13345 / a1b47db
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/artifact/patchprocess/whitespace-eol.txt
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 7 new or modified test files. +1 mvninstall 6m 49s HADOOP-13345 passed +1 compile 0m 20s HADOOP-13345 passed +1 checkstyle 0m 14s HADOOP-13345 passed +1 mvnsite 0m 26s HADOOP-13345 passed +1 mvneclipse 0m 16s HADOOP-13345 passed +1 findbugs 0m 28s HADOOP-13345 passed +1 javadoc 0m 15s HADOOP-13345 passed +1 mvninstall 0m 19s the patch passed +1 compile 0m 17s the patch passed +1 javac 0m 17s the patch passed -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 7 new + 33 unchanged - 0 fixed = 40 total (was 33) +1 mvnsite 0m 23s the patch passed +1 mvneclipse 0m 13s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 0m 33s the patch passed -1 javadoc 0m 11s hadoop-aws in the patch failed. +1 unit 0m 33s hadoop-aws in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 13m 14s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12846061/HADOOP-13336-HADOOP-13345-002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 785d1ba1db19 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision HADOOP-13345 / a1b47db Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/artifact/patchprocess/whitespace-eol.txt javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11382/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Patch 002. This one I like

          When a new FS instance with uri s3a://BUCKET.whatever is created, the supplied conf is cloned, with all fs.s3a.bucket.BUCKET properties copied onto the base `fs.s3a`. (excluding fs.s3a.impl and any attempts to overwrite those with fs.s3a.bucket).

          This lets you do things like declare different endpoints for different buckets:

            <property>
              <name>fs.s3a.bucket.landsat-pds.endpoint</name>
              <value>s3.amazonaws.com</value>
              <description>The endpoint for s3a://landsat-pds URLs</description>
            </property>
          

          It will also handle: auth mechanisms, fadvice policy, output tuning, etc, etc, so support: different buckets with different access accounts, remote locales, etc.

          Test: yes, of base propagation.
          I've added an implicit one by removing the special code needed to let you specify a different endpoint for the test CSV file. Now, you can change the default fs.s3a.endpoint to somewhere like frankfurt, yet still use the landsat image, just by defining the new endpoint for this.

          Tested against s3a frankfurt, without the override (To verify the default endpoint is picked up), then again with the overridden endpoint.

          Documentation. Yes, with examples covering endpoints and authentication. I also cut the section on CSV endpoint configuration, as its implicitly covered by the new stuff.

          Show
          stevel@apache.org Steve Loughran added a comment - Patch 002. This one I like When a new FS instance with uri s3a://BUCKET.whatever is created, the supplied conf is cloned, with all fs.s3a.bucket.BUCKET properties copied onto the base `fs.s3a`. (excluding fs.s3a.impl and any attempts to overwrite those with fs.s3a.bucket). This lets you do things like declare different endpoints for different buckets: <property> <name>fs.s3a.bucket.landsat-pds.endpoint</name> <value>s3.amazonaws.com</value> <description>The endpoint for s3a: //landsat-pds URLs</description> </property> It will also handle: auth mechanisms, fadvice policy, output tuning, etc, etc, so support: different buckets with different access accounts, remote locales, etc. Test: yes, of base propagation. I've added an implicit one by removing the special code needed to let you specify a different endpoint for the test CSV file. Now, you can change the default fs.s3a.endpoint to somewhere like frankfurt, yet still use the landsat image, just by defining the new endpoint for this. Tested against s3a frankfurt, without the override (To verify the default endpoint is picked up), then again with the overridden endpoint. Documentation. Yes, with examples covering endpoints and authentication. I also cut the section on CSV endpoint configuration, as its implicitly covered by the new stuff.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 8 new or modified test files.
          +1 mvninstall 12m 28s HADOOP-13345 passed
          +1 compile 0m 22s HADOOP-13345 passed
          +1 checkstyle 0m 17s HADOOP-13345 passed
          +1 mvnsite 0m 31s HADOOP-13345 passed
          +1 mvneclipse 2m 14s HADOOP-13345 passed
          +1 findbugs 0m 35s HADOOP-13345 passed
          +1 javadoc 0m 18s HADOOP-13345 passed
          +1 mvninstall 0m 20s the patch passed
          +1 compile 0m 18s the patch passed
          +1 javac 0m 18s the patch passed
          -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 15 new + 15 unchanged - 1 fixed = 30 total (was 16)
          +1 mvnsite 0m 24s the patch passed
          +1 mvneclipse 0m 15s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 41s the patch passed
          -1 javadoc 0m 13s hadoop-aws in the patch failed.
          +1 unit 0m 40s hadoop-aws in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          21m 44s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13336
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12845855/HADOOP-13336-HADOOP-13345-001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 32656a06af8f 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision HADOOP-13345 / a412b10
          Default Java 1.8.0_111
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11375/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/11375/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11375/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11375/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 8 new or modified test files. +1 mvninstall 12m 28s HADOOP-13345 passed +1 compile 0m 22s HADOOP-13345 passed +1 checkstyle 0m 17s HADOOP-13345 passed +1 mvnsite 0m 31s HADOOP-13345 passed +1 mvneclipse 2m 14s HADOOP-13345 passed +1 findbugs 0m 35s HADOOP-13345 passed +1 javadoc 0m 18s HADOOP-13345 passed +1 mvninstall 0m 20s the patch passed +1 compile 0m 18s the patch passed +1 javac 0m 18s the patch passed -0 checkstyle 0m 12s hadoop-tools/hadoop-aws: The patch generated 15 new + 15 unchanged - 1 fixed = 30 total (was 16) +1 mvnsite 0m 24s the patch passed +1 mvneclipse 0m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 41s the patch passed -1 javadoc 0m 13s hadoop-aws in the patch failed. +1 unit 0m 40s hadoop-aws in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 21m 44s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13336 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12845855/HADOOP-13336-HADOOP-13345-001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 32656a06af8f 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision HADOOP-13345 / a412b10 Default Java 1.8.0_111 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11375/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/11375/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11375/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11375/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          HADOOP-13336 patch 001. This adds a new BucketConfiguration class which exports some of the classic Configuration calls, but also pulls in some of the extension methods from S3AUtils. Moved to across s3 and s3guard. All existing tests are working without any actual changes to specific buckets. Those tests are TODO.

          Now, looking at what DFSUtils have done, I can't help thinking I've done it completely wrong. Instead of having the look & fallback, I should just do what is done there with propagation: just take the config for an FS and patch in all the properties from the bucket to the toplevel values. That's harder to see what's gone wrong, but means that actually a lot of this patch complexity isn't needed: no new type or anything. And we can add a log @ debug of what propagation takes place.

          let me do that tomorrow. At least now I know my way round what s3guard does better

          Show
          stevel@apache.org Steve Loughran added a comment - HADOOP-13336 patch 001. This adds a new BucketConfiguration class which exports some of the classic Configuration calls, but also pulls in some of the extension methods from S3AUtils. Moved to across s3 and s3guard. All existing tests are working without any actual changes to specific buckets. Those tests are TODO. Now, looking at what DFSUtils have done, I can't help thinking I've done it completely wrong. Instead of having the look & fallback, I should just do what is done there with propagation: just take the config for an FS and patch in all the properties from the bucket to the toplevel values. That's harder to see what's gone wrong, but means that actually a lot of this patch complexity isn't needed: no new type or anything. And we can add a log @ debug of what propagation takes place. let me do that tomorrow. At least now I know my way round what s3guard does better
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for the plan described in Steve's last comment.

          Longer term, I wonder if we can find some commonality with other configuration prefix qualification use cases in the codebase, e.g. the DFSUtil methods for qualifying NameNode address configuration by nameservice in an HA or federated deployment. Perhaps all such use cases could be handled by common utilities. No need to worry about this in scope of this JIRA though. We can always refactor later if we find commonality.

          Show
          cnauroth Chris Nauroth added a comment - +1 for the plan described in Steve's last comment. Longer term, I wonder if we can find some commonality with other configuration prefix qualification use cases in the codebase, e.g. the DFSUtil methods for qualifying NameNode address configuration by nameservice in an HA or federated deployment. Perhaps all such use cases could be handled by common utilities. No need to worry about this in scope of this JIRA though. We can always refactor later if we find commonality.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I'm about to do this; I'll pull it into the s3guard branch first, so that we can tune it there, where the issues related to different dynamodb policies surface.

          Proposed:

          1. Ignore the issue of bucket names with "." in them. There's already an assumption in the code that hostname==bucket name.
          2. then use the bucket name as an fs3.s3a.bucket.* prefix
          3. falling back to fs.s3a. * for properties that aren't in a bucket

          What I like about this is that a list of all properties and grep fs.s3a.bucket.mybucket will find all custom settings for a given bucket. Do it trailing and your management tooling/text editor has a harder time identifying what has changed. I think it gives us more option extensibility too.

          I'll add an extended configuration class which does the lookup and fallback, for all the methods currently in use in S3A. That is, getTrimmed, getPassword, getBytes, ... Self contained, with its own tests. Then move s3A and things it creates (output streams, ...) to it. The S3aFS would create the config from its config & bucket name, pass that instance down to the other things instead of a Configuration.

          Show
          stevel@apache.org Steve Loughran added a comment - I'm about to do this; I'll pull it into the s3guard branch first, so that we can tune it there, where the issues related to different dynamodb policies surface. Proposed: Ignore the issue of bucket names with "." in them. There's already an assumption in the code that hostname==bucket name. then use the bucket name as an fs3.s3a.bucket.* prefix falling back to fs.s3a. * for properties that aren't in a bucket What I like about this is that a list of all properties and grep fs.s3a.bucket.mybucket will find all custom settings for a given bucket. Do it trailing and your management tooling/text editor has a harder time identifying what has changed. I think it gives us more option extensibility too. I'll add an extended configuration class which does the lookup and fallback, for all the methods currently in use in S3A . That is, getTrimmed, getPassword, getBytes, ... Self contained, with its own tests. Then move s3A and things it creates (output streams, ...) to it. The S3aFS would create the config from its config & bucket name, pass that instance down to the other things instead of a Configuration.
          Hide
          liuml07 Mingliang Liu added a comment -

          +1 on Option A.

          I'd suggest a prefix of fs.s3a.bucket.<bucket-name> instead of fs.s3a.<bucket-name>

          +1 on this as well.

          Show
          liuml07 Mingliang Liu added a comment - +1 on Option A. I'd suggest a prefix of fs.s3a.bucket.<bucket-name> instead of fs.s3a.<bucket-name> +1 on this as well.
          Hide
          fabbri Aaron Fabbri added a comment -

          Putting the bucket name at the end seems more correct, thanks.

          I like how explicit using fs.s3a.default.* namespace is, but it breaks compatibility with the existing config keys. I think we either need to omit the .default part or deal with supporting both and deprecation, etc. I'd lean towards just omitting the .default: it is implicit when .bucket is absent.

          Show
          fabbri Aaron Fabbri added a comment - Putting the bucket name at the end seems more correct, thanks. I like how explicit using fs.s3a.default.* namespace is, but it breaks compatibility with the existing config keys. I think we either need to omit the .default part or deal with supporting both and deprecation, etc. I'd lean towards just omitting the .default : it is implicit when .bucket is absent.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          thanks for this

          I was wondering until that last sentence why you were proposing something bucket-last (little endian?) until you got to that bit about names containing the "." character.

          I'm not actually sure that Hadoop copes with "." in a name; certainly S3A assume that it's the host: bucket = name.getHost();, and as that's the bucket name used in generating upload requests, you won't be able to PUT any data in them. Nobody has ever complained to us about that yet.

          What about from a "fielding support calls" perspective? I think we'd need a way to get a list of all config options which have been picked up by a bucket, otherwise a single typo would result in the default value being picked up instead.

          Show
          stevel@apache.org Steve Loughran added a comment - thanks for this I was wondering until that last sentence why you were proposing something bucket-last (little endian?) until you got to that bit about names containing the "." character. I'm not actually sure that Hadoop copes with "." in a name; certainly S3A assume that it's the host: bucket = name.getHost(); , and as that's the bucket name used in generating upload requests, you won't be able to PUT any data in them. Nobody has ever complained to us about that yet . What about from a "fielding support calls" perspective? I think we'd need a way to get a list of all config options which have been picked up by a bucket, otherwise a single typo would result in the default value being picked up instead.
          Hide
          dlaurence Doug Laurence added a comment -

          It's good to support configuration that is simple for the simple cases, and more complex (but possible) for the more sophisticated use cases. A single bucket in one region is the simplest case, and multiple buckets in multiple regions is on the more sophisticated end of the spectrum. Providing attributes in the URI is really simple, but probably breaks down over time as capabilities/features are introduced over time, so the fs.s3a config set approach seems more future-proof.

          For example, a user with two buckets (e.g. landsat1 and landsat2) in the same region (e.g. us-west-2) could set the endpoint with a single config set property:

          fs.s3a.default.endpoint=s3-us-west-2.amazonaws.com

          and then simply use the URI prefixes s3a://landsat1/ and s3a://landsat2/

          If a third bucket (e.g. landsat-frankfurt) needed a different endpoint, then a per-bucket override would be required:

          fs.s3a.bucket.endpoint.landsat-frankfurt=s3.eu-central-1.amazonaws.com

          This allows for specifying default properties for all buckets and properties for individual buckets such as authentication details, but also enables the addition of new features over time if desired.

          For example, if we wanted to add support for specifying the default S3 storage class (e.g. S3-Infrequent Access) when each new file is created, we could add a new storage-class sub-property to the config sets i.e. fs.s3a.default.storage-class and fs.s3a.bucket.storage-class. For example:

          fs.s3a.bucket.storage-class.landsat-frankfurt=STANDARD_IA

          or we could enable users to specify object tag(s) to be applied to each new object created:

          fs.s3a.bucket.object-tagging.landsat-frankfurt=Project=x,Classification=internal

          In general, the per-bucket config set sub-properties would also be supported in the fs.s3a.default.* config set namespace. I suggest making the bucket name the suffix in the property namespace since bucket names can contain the '.' character.

          Show
          dlaurence Doug Laurence added a comment - It's good to support configuration that is simple for the simple cases, and more complex (but possible) for the more sophisticated use cases. A single bucket in one region is the simplest case, and multiple buckets in multiple regions is on the more sophisticated end of the spectrum. Providing attributes in the URI is really simple, but probably breaks down over time as capabilities/features are introduced over time, so the fs.s3a config set approach seems more future-proof. For example, a user with two buckets (e.g. landsat1 and landsat2) in the same region (e.g. us-west-2) could set the endpoint with a single config set property: fs.s3a. default .endpoint=s3-us-west-2.amazonaws.com and then simply use the URI prefixes s3a://landsat1/ and s3a://landsat2/ If a third bucket (e.g. landsat-frankfurt) needed a different endpoint, then a per-bucket override would be required: fs.s3a.bucket.endpoint.landsat-frankfurt=s3.eu-central-1.amazonaws.com This allows for specifying default properties for all buckets and properties for individual buckets such as authentication details, but also enables the addition of new features over time if desired. For example, if we wanted to add support for specifying the default S3 storage class (e.g. S3-Infrequent Access) when each new file is created, we could add a new storage-class sub-property to the config sets i.e. fs.s3a.default.storage-class and fs.s3a.bucket.storage-class. For example: fs.s3a.bucket.storage-class.landsat-frankfurt=STANDARD_IA or we could enable users to specify object tag(s) to be applied to each new object created: fs.s3a.bucket.object-tagging.landsat-frankfurt=Project=x,Classification=internal In general, the per-bucket config set sub-properties would also be supported in the fs.s3a.default.* config set namespace. I suggest making the bucket name the suffix in the property namespace since bucket names can contain the '.' character.
          Hide
          mackrorysd Sean Mackrory added a comment -

          Oh yeah - I'm 100% on board with that not being supported anymore - just saying we need to clearly call that out as an incompatibility. I guarantee it's going to break SOMEONE'S job.

          Show
          mackrorysd Sean Mackrory added a comment - Oh yeah - I'm 100% on board with that not being supported anymore - just saying we need to clearly call that out as an incompatibility. I guarantee it's going to break SOMEONE'S job.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          ps, you know that once you start declaring your own auth chain, we take away the ability to use id:secret in URIs. It's just near-impossible to keep those secrets out the logs, despite all the effort in HADOOP-3733. sticking your secrets in URIs is just a way to lose your data and your money

          Show
          stevel@apache.org Steve Loughran added a comment - ps, you know that once you start declaring your own auth chain, we take away the ability to use id:secret in URIs. It's just near-impossible to keep those secrets out the logs, despite all the effort in HADOOP-3733 . sticking your secrets in URIs is just a way to lose your data and your money
          Hide
          stevel@apache.org Steve Loughran added a comment -

          yeah, you can't use key:secret@bucket for config. But here's the thing: you shouldn't be doing that for security reasons. the sole justification today is that it's the only way to do work across accounts. With different config sets for different buckets, that use case changes

          and yes, it was implicit in my thought's that username only -> config, user:pass -> credentials.

          Show
          stevel@apache.org Steve Loughran added a comment - yeah, you can't use key:secret@bucket for config. But here's the thing: you shouldn't be doing that for security reasons. the sole justification today is that it's the only way to do work across accounts. With different config sets for different buckets, that use case changes and yes, it was implicit in my thought's that username only -> config, user:pass -> credentials.
          Hide
          mackrorysd Sean Mackrory added a comment -

          Worth pointing out that option C would also break compatibility for some folks. If they're currently using s3a://acccess-key:secret-key@bucket/, that'll change. We could say that if they provide a username and password or if the username is not a valid configuration domain (unlikely for someone to be using an access key id as configuration keys, I think) that we interpret it the current way. But that's a little clunky.

          Show
          mackrorysd Sean Mackrory added a comment - Worth pointing out that option C would also break compatibility for some folks. If they're currently using s3a://acccess-key:secret-key@bucket/, that'll change. We could say that if they provide a username and password or if the username is not a valid configuration domain (unlikely for someone to be using an access key id as configuration keys, I think) that we interpret it the current way. But that's a little clunky.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Also, does this break folks who use FQDN bucket names?

          yes, hence my lack of enthusiasm.

          Show
          stevel@apache.org Steve Loughran added a comment - Also, does this break folks who use FQDN bucket names? yes, hence my lack of enthusiasm.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          note we don't need to change existing URIs, just new ones, if they have a different config from the default. Use the default values, you get what's normal. It's just no longer transparent what you've got

          Oh, one little issue: on 2.7-2.8, if you include a user in the authority, but no password, e.g. no : or details, you just get login information ignored silently. We may want to urgently change 2.8 and 2.7.x to fail here, even before we settle on what a good config policy is.

          Show
          stevel@apache.org Steve Loughran added a comment - note we don't need to change existing URIs, just new ones, if they have a different config from the default. Use the default values, you get what's normal. It's just no longer transparent what you've got Oh, one little issue: on 2.7-2.8, if you include a user in the authority, but no password, e.g. no : or details, you just get login information ignored silently. We may want to urgently change 2.8 and 2.7.x to fail here, even before we settle on what a good config policy is.
          Hide
          fabbri Aaron Fabbri added a comment -

          Just noticed you suggest the same thing about having a ".bucket.<bucket-name>" prefix, so disregard that comment (it was just missing from landsat example).

          Show
          fabbri Aaron Fabbri added a comment - Just noticed you suggest the same thing about having a ".bucket.<bucket-name>" prefix, so disregard that comment (it was just missing from landsat example).
          Hide
          fabbri Aaron Fabbri added a comment -

          Great summary Steve Loughran. I think being backward-compatible with existing configs and URIs in production is important. These all seem reasonable, but URI compatibility seems to point to option A for me (if we want to keep it simple). The annoying thing is that these are hard to change if we decide we want a different option. Which option are you leaning towards?

          Option A per-bucket config.
          Lets you define everything for a bucket.
          Examples
          s3a://olap2/data/2017 : s3a URL s3a://olap2/data/2017, with config set fs.s3a.bucket.olap2 in configuration
          s3a://landsat : s3a URL s3a://landsat, with config set fs.s3a.landsat for anonymous credentials and no dynamo

          To avoid key space conflicts I'd suggest a prefix of fs.s3a.bucket.<bucket-name> instead of fs.s3a.<bucket-name>. Just in case someone has an s3 bucket named "endpoint", they'd use fs.s3a.bucket.endpoint.* instead of conflicting with fs.s3a.endpoint, etc..

          This option seems pretty straightforward. Should be backward compatible as it requires no changes to URIs and existing default or "all bucket" config keys continue to work the same. For grabbing config values in S3A, we'd call some per-bucket Configuration wrapper that looks for the fs.s3a.bucket.<bucket-name>.* key first, and if not, returns whatever is in the non-bucket-specific config.

          Option B config via domain name in URL
          This is what swift does: you define a domain, with the domain defining everything.
          s3a://olap2.dynamo/data/2017 with config sett fs.s3a.binding.dynamo
          s3a://landsat.anon with config set fs.s3a.binding.anon for anonymous credentials and no dynamo

          As you mention, my desire for URI backward-compatibility implies we need an additional way to map a bucket to a domain, e.g. fs.s3a.domain.bucket.my-bucket=my-domain. Seems a bit too complex. This buys us the ability to share a config over some set of buckets.

          Also, does this break folks who use FQDN bucket names?

          Option C Config via user:pass property in URL
          This is a bit like Azure, where the FQDN defines the binding, and the username defines the bucket. Here I'm proposing the ability to define a new user which declares the binding info.
          Examples
          s3a://dynamo@olap2/data/2017 : s3a URL s3a://olap2/data/2017, with config set fs.s3a.binding.dynamo
          s3a://anon@landsat : s3a URL s3a://landsat, with config set fs.s3a.binding.anon for anonymous credentials.

          Seems reasonable but the need to change URIs is unfortunate.

          Show
          fabbri Aaron Fabbri added a comment - Great summary Steve Loughran . I think being backward-compatible with existing configs and URIs in production is important. These all seem reasonable, but URI compatibility seems to point to option A for me (if we want to keep it simple). The annoying thing is that these are hard to change if we decide we want a different option. Which option are you leaning towards? Option A per-bucket config. Lets you define everything for a bucket. Examples s3a://olap2/data/2017 : s3a URL s3a://olap2/data/2017, with config set fs.s3a.bucket.olap2 in configuration s3a://landsat : s3a URL s3a://landsat, with config set fs.s3a.landsat for anonymous credentials and no dynamo To avoid key space conflicts I'd suggest a prefix of fs.s3a.bucket.<bucket-name> instead of fs.s3a.<bucket-name>. Just in case someone has an s3 bucket named "endpoint", they'd use fs.s3a.bucket.endpoint.* instead of conflicting with fs.s3a.endpoint , etc.. This option seems pretty straightforward. Should be backward compatible as it requires no changes to URIs and existing default or "all bucket" config keys continue to work the same. For grabbing config values in S3A, we'd call some per-bucket Configuration wrapper that looks for the fs.s3a.bucket.<bucket-name>.* key first, and if not, returns whatever is in the non-bucket-specific config. Option B config via domain name in URL This is what swift does: you define a domain, with the domain defining everything. s3a://olap2.dynamo/data/2017 with config sett fs.s3a.binding.dynamo s3a://landsat.anon with config set fs.s3a.binding.anon for anonymous credentials and no dynamo As you mention, my desire for URI backward-compatibility implies we need an additional way to map a bucket to a domain, e.g. fs.s3a.domain.bucket.my-bucket=my-domain . Seems a bit too complex. This buys us the ability to share a config over some set of buckets. Also, does this break folks who use FQDN bucket names? Option C Config via user:pass property in URL This is a bit like Azure, where the FQDN defines the binding, and the username defines the bucket. Here I'm proposing the ability to define a new user which declares the binding info. Examples s3a://dynamo@olap2/data/2017 : s3a URL s3a://olap2/data/2017, with config set fs.s3a.binding.dynamo s3a://anon@landsat : s3a URL s3a://landsat, with config set fs.s3a.binding.anon for anonymous credentials. Seems reasonable but the need to change URIs is unfortunate.
          Hide
          stevel@apache.org Steve Loughran added a comment - - edited

          This also matters for HADOOP-13345, where different buckets will have different MD caching policies, including "none", so increasing its priority.

          Possibilities —all of which assume falling back to the s3a standard options as default. This means: no way to undefine an option.

          per-bucket config.

          Lets you define everything for a bucket.

          Examples

          • s3a://olap2/data/2017 : s3a URL s3a://olap2/data/2017, with config set fs.s3a.bucket.olap2 in configuration
          • s3a://landsat : s3a URL s3a://landsat, with config set fs.s3a.bucket.landsat for anonymous credentials and no dynamo

          Pro

          • Conceptually simple
          • easy to get started
          • trivial to move between other s3 clients, just change the prefix/redeclare the prefix binding

          Con

          • Expensive/complicated to maintain configurations.
          • Need to delve into the configuration file to see what the mappings are. I can see this mattering a lot in support calls related to authentication.

          config via domain name in URL

          This is what swift does: you define a domain, with the domain defining everything.

          • s3a://olap2.dynamo/data/2017 with config sett fs.s3a.binding.dynamo
          • s3a://landsat.anon with config set fs.s3a.binding.anon for anonymous credentials and no dynamo

          Pro:

          • shared config across multiple buckets
          • easy to see when buckets have different config options without having delve into the configuration file to see what the mappings are.
          • Matches swift://
          • Similar-ish to wasb

          Con:

          • the need to explicitly declare a domain stops you transparently moving a bucket to a different set of options, unless you add a way to also bind a bucket to a "configuration domain", behind the scenes.
          • S3 supports FQDNs already
          • not going to be compatible with previous versions, external s3 clients, (e.g. EMR)

          Config via user:pass property in URL

          This is a bit like Azure, where the FQDN defines the binding, and the username defines the bucket. Here I'm proposing the ability to define a new user which declares the binding info.

          Examples

          • s3a://dynamo@olap2/data/2017 : s3a URL s3a://olap2/data/2017, with config set fs.s3a.binding.dynamo
          • s3a://anon@landsat : s3a URL s3a://landsat, with config set fs.s3a.binding.anon for anonymous credentials.

          Pro:

          • Better for sharing configuration options across buckets
          • consistent model with the AWSID:secret mechanism today
          • see at a glance what the configuration set used is, easy to change.
          • no complications related to domain naming
          • Easy to switch between configuration sets on the command line, without adding new properties.

          Con:

          • needs different URLs if you don't want the default.

          Fundamentally rework Hadoop configuration to support a hierarchical configuration mechanism.

          I'm not really proposing this, just wanted to mention it as the nominal ultimate option, instead of what we have today with different things (HA, Swift, Azure, etc), all defining different mechanisms for tuning customisation.

          (2016-12-10: updated by fixing landsat config option name in the per-bucket-config example)

          Show
          stevel@apache.org Steve Loughran added a comment - - edited This also matters for HADOOP-13345 , where different buckets will have different MD caching policies, including "none", so increasing its priority. Possibilities —all of which assume falling back to the s3a standard options as default. This means: no way to undefine an option. per-bucket config. Lets you define everything for a bucket. Examples s3a://olap2/data/2017 : s3a URL s3a://olap2/data/2017 , with config set fs.s3a.bucket.olap2 in configuration s3a://landsat : s3a URL s3a://landsat , with config set fs.s3a.bucket.landsat for anonymous credentials and no dynamo Pro Conceptually simple easy to get started trivial to move between other s3 clients, just change the prefix/redeclare the prefix binding Con Expensive/complicated to maintain configurations. Need to delve into the configuration file to see what the mappings are. I can see this mattering a lot in support calls related to authentication. config via domain name in URL This is what swift does: you define a domain, with the domain defining everything. s3a://olap2.dynamo/data/2017 with config sett fs.s3a.binding.dynamo s3a://landsat.anon with config set fs.s3a.binding.anon for anonymous credentials and no dynamo Pro: shared config across multiple buckets easy to see when buckets have different config options without having delve into the configuration file to see what the mappings are. Matches swift:// Similar-ish to wasb Con: the need to explicitly declare a domain stops you transparently moving a bucket to a different set of options, unless you add a way to also bind a bucket to a "configuration domain", behind the scenes. S3 supports FQDNs already not going to be compatible with previous versions, external s3 clients, (e.g. EMR) Config via user:pass property in URL This is a bit like Azure, where the FQDN defines the binding, and the username defines the bucket. Here I'm proposing the ability to define a new user which declares the binding info. Examples s3a://dynamo@olap2/data/2017 : s3a URL s3a://olap2/data/2017 , with config set fs.s3a.binding.dynamo s3a://anon@landsat : s3a URL s3a://landsat , with config set fs.s3a.binding.anon for anonymous credentials. Pro: Better for sharing configuration options across buckets consistent model with the AWSID:secret mechanism today see at a glance what the configuration set used is, easy to change. no complications related to domain naming Easy to switch between configuration sets on the command line, without adding new properties. Con: needs different URLs if you don't want the default. Fundamentally rework Hadoop configuration to support a hierarchical configuration mechanism. I'm not really proposing this, just wanted to mention it as the nominal ultimate option, instead of what we have today with different things (HA, Swift, Azure, etc), all defining different mechanisms for tuning customisation. (2016-12-10: updated by fixing landsat config option name in the per-bucket-config example)
          Hide
          stevel@apache.org Steve Loughran added a comment -

          There's another config strategy, where we configure an endpoint and then assign buckets to them by way of domains

          fs.s3a.endpoint.eu.address=frankfurt.s3.aws.com
          fs.s3a.endpoint.eu.awsid=AWSID
          fs.s3a.endpoint.eu.secret=AWSSECRET
          

          Then you'd refer to a bucket by bucket.endpoint: s3a://stevel.frankfurt

          This is how openstack is configured. Where it might cause problems is for s3-compatible installations in use today, where an FQDN is used

          Show
          stevel@apache.org Steve Loughran added a comment - There's another config strategy, where we configure an endpoint and then assign buckets to them by way of domains fs.s3a.endpoint.eu.address=frankfurt.s3.aws.com fs.s3a.endpoint.eu.awsid=AWSID fs.s3a.endpoint.eu.secret=AWSSECRET Then you'd refer to a bucket by bucket.endpoint: s3a://stevel.frankfurt This is how openstack is configured. Where it might cause problems is for s3-compatible installations in use today, where an FQDN is used
          Hide
          cnauroth Chris Nauroth added a comment -

          What I'm thinking of here is allowing you to override all global options with per-bucket values;

          +1. I've had some other use cases in mind for this too:

          • Different endpoint settings.
          • Different TCP connection management settings and multi-part settings for a process co-located in the same AWS region as one bucket but not the other.
          • Different User-Agent customization within the same process.
          • Server-side encryption on one bucket but not the other.

          Thanks for coming up with a generalized configuration idea.

          Show
          cnauroth Chris Nauroth added a comment - What I'm thinking of here is allowing you to override all global options with per-bucket values; +1. I've had some other use cases in mind for this too: Different endpoint settings. Different TCP connection management settings and multi-part settings for a process co-located in the same AWS region as one bucket but not the other. Different User-Agent customization within the same process. Server-side encryption on one bucket but not the other. Thanks for coming up with a generalized configuration idea.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          What I'm thinking of here is allowing you to override all global options with per-bucket values; this would also let you use different authentication options for different buckets

          e.g

          fs.s3a.bucket.steve-frankfurt.endpoint=frankfurt
          fs.s3a.bucket.steve-frankfurt.awsid=my-frankfurt-id
          ...
          

          when an s3a instance is built up, it'd read the globals, anything localized would take priority.

          Side effect: confusion and diagnostics just got more complex

          Show
          stevel@apache.org Steve Loughran added a comment - What I'm thinking of here is allowing you to override all global options with per-bucket values; this would also let you use different authentication options for different buckets e.g fs.s3a.bucket.steve-frankfurt.endpoint=frankfurt fs.s3a.bucket.steve-frankfurt.awsid=my-frankfurt-id ... when an s3a instance is built up, it'd read the globals, anything localized would take priority. Side effect: confusion and diagnostics just got more complex
          Hide
          stevel@apache.org Steve Loughran added a comment -

          problem is that with V4 auth you have to spec the endpoint, so you can't do a cp from frankfurt to s korea...or just from us-east to frankfurt

          Show
          stevel@apache.org Steve Loughran added a comment - problem is that with V4 auth you have to spec the endpoint, so you can't do a cp from frankfurt to s korea...or just from us-east to frankfurt
          Hide
          slider Steven K. Wong added a comment -

          S3A already supports cross-region, if your config doesn't specify endpoint and doesn't enable path-style access. I am able to hadoop fs -cp a file from an S3 bucket in one region (us-east-1) to another S3 bucket in a different region (us-west-1).

          Show
          slider Steven K. Wong added a comment - S3A already supports cross-region, if your config doesn't specify endpoint and doesn't enable path-style access. I am able to hadoop fs -cp a file from an S3 bucket in one region (us-east-1) to another S3 bucket in a different region (us-west-1).

            People

            • Assignee:
              stevel@apache.org Steve Loughran
              Reporter:
              stevel@apache.org Steve Loughran
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development