Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.6.0
    • Fix Version/s: 2.9.0, 3.0.0-alpha4
    • Component/s: webhdfs
    • Labels:
      None

      Description

      Part of HDFS-5608 added support for GET/SET ACLs over WebHDFS. This currently identifies the passed arguments via a hard-coded regex that mandates certain group and user naming styles.

      A similar limitation had existed before for CHOWN and other User/Group set related operations of WebHDFS, where it was then made configurable via HDFS-11391 + HDFS-4983.

      Such configurability should be allowed for the ACL operations too.

      1. HDFS-11421.000.patch
        13 kB
        Harsh J
      2. HDFS-11421.branch-2.001.patch
        14 kB
        Xiao Chen
      3. HDFS-11421.branch-2.003.patch
        14 kB
        Xiao Chen
      4. HDFS-11421-branch-2.000.patch
        13 kB
        Harsh J

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user QwertyManiac opened a pull request:

          https://github.com/apache/hadoop/pull/195

          HDFS-11421. Make WebHDFS' ACLs RegEx configurable. (harsh)

          • Introduced a new config key `dfs.webhdfs.acl.provider.permission.pattern` with value set to existing default ACL regex
          • Added pattern-from-config setting points for `AclPermissionParam` to `NameNodeHttpServer`, `WebHdfsFileSystem` and `WebHdfsHandler`, akin to how `UserParam` is set today at these three places
          • Added setters/getters for custom pattern to `AclPermissionParam`
          • Added a new test for `AclPermissionParam` custom patterns that exercises numeric usernames and group names with `@` characters
          • Extended existing `UserParam` configurability tests to cover ACL modification with numeric usernames and group names with `@` characters over a custom pattern
          • Ran existing relevant tests, which continue to pass

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/QwertyManiac/hadoop HDFS-11421

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/hadoop/pull/195.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #195


          commit f80343d972342033db7157dcf39a89bdb98ebf16
          Author: Harsh J <harsh@cloudera.com>
          Date: 2017-02-16T12:47:07Z

          HDFS-11421. Make WebHDFS' ACLs RegEx configurable. (harsh)


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user QwertyManiac opened a pull request: https://github.com/apache/hadoop/pull/195 HDFS-11421 . Make WebHDFS' ACLs RegEx configurable. (harsh) Introduced a new config key `dfs.webhdfs.acl.provider.permission.pattern` with value set to existing default ACL regex Added pattern-from-config setting points for `AclPermissionParam` to `NameNodeHttpServer`, `WebHdfsFileSystem` and `WebHdfsHandler`, akin to how `UserParam` is set today at these three places Added setters/getters for custom pattern to `AclPermissionParam` Added a new test for `AclPermissionParam` custom patterns that exercises numeric usernames and group names with `@` characters Extended existing `UserParam` configurability tests to cover ACL modification with numeric usernames and group names with `@` characters over a custom pattern Ran existing relevant tests, which continue to pass You can merge this pull request into a Git repository by running: $ git pull https://github.com/QwertyManiac/hadoop HDFS-11421 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/hadoop/pull/195.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #195 commit f80343d972342033db7157dcf39a89bdb98ebf16 Author: Harsh J <harsh@cloudera.com> Date: 2017-02-16T12:47:07Z HDFS-11421 . Make WebHDFS' ACLs RegEx configurable. (harsh)
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks for reporting and fixing this Harsh J!

          Looks pretty good to me, just some nits in TestParam:

          • Can we verify the actual value, rather than assertNotNull?
          • Seems we can let the exception to throw and get logged automatically by junit, instead of catching and assert fail.
          • This exists before your patch, but maybe AclPermissionParam.setAclPermissionPattern(oldDomain); in a finally block?

          And this is my first review of a github PR, do you know how to trigger pre-commits? https://wiki.apache.org/hadoop/HowToContribute#Creating_a_GitHub_pull_request says it's automatic, I just kicked a jenkins run and let's see if it is.

          Show
          xiaochen Xiao Chen added a comment - Thanks for reporting and fixing this Harsh J ! Looks pretty good to me, just some nits in TestParam : Can we verify the actual value, rather than assertNotNull ? Seems we can let the exception to throw and get logged automatically by junit, instead of catching and assert fail. This exists before your patch, but maybe AclPermissionParam.setAclPermissionPattern(oldDomain); in a finally block? And this is my first review of a github PR, do you know how to trigger pre-commits? https://wiki.apache.org/hadoop/HowToContribute#Creating_a_GitHub_pull_request says it's automatic, I just kicked a jenkins run and let's see if it is.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 25s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 13s Maven dependency ordering for branch
          +1 mvninstall 13m 53s trunk passed
          +1 compile 1m 24s trunk passed
          +1 checkstyle 0m 40s trunk passed
          +1 mvnsite 1m 25s trunk passed
          +1 mvneclipse 0m 25s trunk passed
          +1 findbugs 3m 13s trunk passed
          +1 javadoc 1m 3s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 1m 24s the patch passed
          +1 compile 1m 23s the patch passed
          +1 javac 1m 23s hadoop-hdfs-project generated 0 new + 78 unchanged - 2 fixed = 78 total (was 80)
          +1 checkstyle 0m 38s the patch passed
          +1 mvnsite 1m 21s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 3m 42s the patch passed
          +1 javadoc 1m 2s the patch passed
          +1 unit 1m 2s hadoop-hdfs-client in the patch passed.
          -1 unit 70m 57s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          106m 36s



          Reason Tests
          Failed junit tests hadoop.metrics2.sink.TestRollingFileSystemSinkWithHdfs
            hadoop.hdfs.server.datanode.metrics.TestDataNodeOutlierDetectionViaMetrics



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HDFS-11421
          GITHUB PR https://github.com/apache/hadoop/pull/195
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux a5d9782ef3ab 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 132f758
          Default Java 1.8.0_121
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/18430/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18430/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18430/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 25s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 13s Maven dependency ordering for branch +1 mvninstall 13m 53s trunk passed +1 compile 1m 24s trunk passed +1 checkstyle 0m 40s trunk passed +1 mvnsite 1m 25s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 3m 13s trunk passed +1 javadoc 1m 3s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 1m 24s the patch passed +1 compile 1m 23s the patch passed +1 javac 1m 23s hadoop-hdfs-project generated 0 new + 78 unchanged - 2 fixed = 78 total (was 80) +1 checkstyle 0m 38s the patch passed +1 mvnsite 1m 21s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 42s the patch passed +1 javadoc 1m 2s the patch passed +1 unit 1m 2s hadoop-hdfs-client in the patch passed. -1 unit 70m 57s hadoop-hdfs in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 106m 36s Reason Tests Failed junit tests hadoop.metrics2.sink.TestRollingFileSystemSinkWithHdfs   hadoop.hdfs.server.datanode.metrics.TestDataNodeOutlierDetectionViaMetrics Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11421 GITHUB PR https://github.com/apache/hadoop/pull/195 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux a5d9782ef3ab 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 132f758 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/18430/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18430/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18430/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          qwertymaniac Harsh J added a comment -

          Thank you for the review Xiao Chen! I've updated the PR with a commit addressing your comments. I'm also attaching a patch form directly here.

          Show
          qwertymaniac Harsh J added a comment - Thank you for the review Xiao Chen ! I've updated the PR with a commit addressing your comments. I'm also attaching a patch form directly here.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Harsh for the new rev! +1 pending jenkins.

          Do you know how to run branch-2's precommit? Seems https://builds.apache.org/job/PreCommit-hdfs-Build/18439/parameters/ is running and HowToContribute says it precommits always run for PR instead of patch file.

          Show
          xiaochen Xiao Chen added a comment - Thanks Harsh for the new rev! +1 pending jenkins. Do you know how to run branch-2's precommit? Seems https://builds.apache.org/job/PreCommit-hdfs-Build/18439/parameters/ is running and HowToContribute says it precommits always run for PR instead of patch file.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 7s Maven dependency ordering for branch
          +1 mvninstall 15m 19s trunk passed
          +1 compile 1m 49s trunk passed
          +1 checkstyle 0m 48s trunk passed
          +1 mvnsite 1m 57s trunk passed
          +1 mvneclipse 0m 34s trunk passed
          +1 findbugs 3m 35s trunk passed
          +1 javadoc 1m 5s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 1m 36s the patch passed
          +1 compile 1m 37s the patch passed
          +1 javac 1m 37s hadoop-hdfs-project generated 0 new + 78 unchanged - 2 fixed = 78 total (was 80)
          +1 checkstyle 0m 41s the patch passed
          +1 mvnsite 1m 28s the patch passed
          +1 mvneclipse 0m 24s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 3m 47s the patch passed
          +1 javadoc 1m 3s the patch passed
          +1 unit 1m 4s hadoop-hdfs-client in the patch passed.
          -1 unit 107m 7s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          146m 39s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFSStriped
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HDFS-11421
          GITHUB PR https://github.com/apache/hadoop/pull/195
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux ab45ed6d3f68 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b32ffa2
          Default Java 1.8.0_121
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/18439/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18439/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18439/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 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 7s Maven dependency ordering for branch +1 mvninstall 15m 19s trunk passed +1 compile 1m 49s trunk passed +1 checkstyle 0m 48s trunk passed +1 mvnsite 1m 57s trunk passed +1 mvneclipse 0m 34s trunk passed +1 findbugs 3m 35s trunk passed +1 javadoc 1m 5s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 1m 36s the patch passed +1 compile 1m 37s the patch passed +1 javac 1m 37s hadoop-hdfs-project generated 0 new + 78 unchanged - 2 fixed = 78 total (was 80) +1 checkstyle 0m 41s the patch passed +1 mvnsite 1m 28s the patch passed +1 mvneclipse 0m 24s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 47s the patch passed +1 javadoc 1m 3s the patch passed +1 unit 1m 4s hadoop-hdfs-client in the patch passed. -1 unit 107m 7s hadoop-hdfs in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 146m 39s Reason Tests Failed junit tests hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFSStriped   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailureReporting Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HDFS-11421 GITHUB PR https://github.com/apache/hadoop/pull/195 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux ab45ed6d3f68 3.13.0-107-generic #154-Ubuntu SMP Tue Dec 20 09:57:27 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b32ffa2 Default Java 1.8.0_121 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/18439/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/18439/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/18439/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Test failures look unrelated and passed locally.

          I just committed this to trunk. Thanks Harsh J for the patch.

          +1 on the branch-2 patch too, but will until we sort out how precommit should run....

          Show
          xiaochen Xiao Chen added a comment - Test failures look unrelated and passed locally. I just committed this to trunk. Thanks Harsh J for the patch. +1 on the branch-2 patch too, but will until we sort out how precommit should run....
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #11305 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11305/)
          HDFS-11421. Make WebHDFS' ACLs RegEx configurable. Contributed by Harsh (xiao: rev e24ed47d9a19f34a4dd8d4bad9b5c78ca3dd1c2e)

          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/WebHdfsHandler.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeHttpServer.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #11305 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11305/ ) HDFS-11421 . Make WebHDFS' ACLs RegEx configurable. Contributed by Harsh (xiao: rev e24ed47d9a19f34a4dd8d4bad9b5c78ca3dd1c2e) (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/web/webhdfs/WebHdfsHandler.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeHttpServer.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/resources/TestParam.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/resources/AclPermissionParam.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHDFS.java
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user QwertyManiac commented on the issue:

          https://github.com/apache/hadoop/pull/195

          Done via e24ed47d9a19f34a4dd8d4bad9b5c78ca3dd1c2e on trunk.

          Show
          githubbot ASF GitHub Bot added a comment - Github user QwertyManiac commented on the issue: https://github.com/apache/hadoop/pull/195 Done via e24ed47d9a19f34a4dd8d4bad9b5c78ca3dd1c2e on trunk.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user QwertyManiac closed the pull request at:

          https://github.com/apache/hadoop/pull/195

          Show
          githubbot ASF GitHub Bot added a comment - Github user QwertyManiac closed the pull request at: https://github.com/apache/hadoop/pull/195
          Hide
          qwertymaniac Harsh J added a comment -

          Xiao Chen - Thanks! I'm uncertain how to trigger just a branch-2 build, but a local build passes with the patch applied, along with the modified tests.

          Show
          qwertymaniac Harsh J added a comment - Xiao Chen - Thanks! I'm uncertain how to trigger just a branch-2 build, but a local build passes with the patch applied, along with the modified tests.
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Hi Harsh J thanks for the branch-2 patch. I think it's good but it doesn't compile.

          I am not sure how to trigger a branch-2 build in this case, but I can help you by running it locally.

          Thanks.

          Show
          jojochuang Wei-Chiu Chuang added a comment - Hi Harsh J thanks for the branch-2 patch. I think it's good but it doesn't compile. I am not sure how to trigger a branch-2 build in this case, but I can help you by running it locally. Thanks.
          Hide
          andrew.wang Andrew Wang added a comment -

          Going to resolve this since it's been pending for a month, please reopen if you plan to get the branch-2 patch committed.

          Show
          andrew.wang Andrew Wang added a comment - Going to resolve this since it's been pending for a month, please reopen if you plan to get the branch-2 patch committed.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks for chiming in, folks.

          I have returned from paternity leave today, sorry for the delay in the past months.

          I think the branch-2 patch is still good. It seems to me the patch applies ok, but failed to compile, due to a line number change and a missing import.

          I'm attaching a modified version of the branch-2 patch, which compiles and passes changed tests locally.

          Given this has been a while, I'd like to be on the safe side and run a more thorough branch-2 test - creating a duplicate jira (that doesn't associate with github) to do so.

          Show
          xiaochen Xiao Chen added a comment - Thanks for chiming in, folks. I have returned from paternity leave today, sorry for the delay in the past months. I think the branch-2 patch is still good. It seems to me the patch applies ok, but failed to compile, due to a line number change and a missing import. I'm attaching a modified version of the branch-2 patch, which compiles and passes changed tests locally. Given this has been a while, I'd like to be on the safe side and run a more thorough branch-2 test - creating a duplicate jira (that doesn't associate with github) to do so.
          Hide
          xiaochen Xiao Chen added a comment -

          Attached branch-2 001 patch, and triggered test via HDFS-11876.

          Harsh J or Wei-Chiu Chuang, could you review when you have time? Thanks.

          Show
          xiaochen Xiao Chen added a comment - Attached branch-2 001 patch, and triggered test via HDFS-11876 . Harsh J or Wei-Chiu Chuang , could you review when you have time? Thanks.
          Hide
          xiaochen Xiao Chen added a comment -

          branch-2 patch 3 has a green jenkins run on HDFS-11876 (With asf license -1 unrelated). So I think it's good to commit.

          But since I made some minor style changes, could you take a look Harsh J? Thanks a lot!

          Show
          xiaochen Xiao Chen added a comment - branch-2 patch 3 has a green jenkins run on HDFS-11876 (With asf license -1 unrelated). So I think it's good to commit. But since I made some minor style changes, could you take a look Harsh J ? Thanks a lot!
          Hide
          qwertymaniac Harsh J added a comment -

          The patch looks good to me. Were the style changes done out of checkstyle warnings? I only notice two changes, one is a comment becoming multi-line, the other's the DOMAIN static member being made lowercased.

          +1

          Show
          qwertymaniac Harsh J added a comment - The patch looks good to me. Were the style changes done out of checkstyle warnings? I only notice two changes, one is a comment becoming multi-line, the other's the DOMAIN static member being made lowercased. +1
          Hide
          xiaochen Xiao Chen added a comment -

          As you said and another >80 char regex string. Committing shortly.

          Show
          xiaochen Xiao Chen added a comment - As you said and another >80 char regex string. Committing shortly.
          Hide
          xiaochen Xiao Chen added a comment -

          Committed to branch-2, thanks Harsh!

          Show
          xiaochen Xiao Chen added a comment - Committed to branch-2, thanks Harsh!

            People

            • Assignee:
              qwertymaniac Harsh J
              Reporter:
              qwertymaniac Harsh J
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development