Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-5659

getPathFromYarnURL should use standard methods

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      getPathFromYarnURL does some string shenanigans where standard ctors should suffice.
      There are also bugs in it e.g. passing an empty scheme to the URI ctor is invalid, null should be used.

      1. YARN-5659.05.patch
        7 kB
        Sergey Shelukhin
      2. YARN-5659.05.patch
        6 kB
        Sergey Shelukhin
      3. YARN-5659.04.patch
        6 kB
        Sergey Shelukhin
      4. YARN-5659.04.patch
        6 kB
        Sergey Shelukhin
      5. YARN-5659.03.patch
        6 kB
        Sergey Shelukhin
      6. YARN-5659.02.patch
        6 kB
        Sergey Shelukhin
      7. YARN-5659.01.patch
        1 kB
        Sergey Shelukhin
      8. YARN-5659.patch
        1 kB
        Sergey Shelukhin

        Activity

        Hide
        sershe Sergey Shelukhin added a comment -

        Thank you for the reviews!

        Show
        sershe Sergey Shelukhin added a comment - Thank you for the reviews!
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10564 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10564/)
        YARN-5659. getPathFromYarnURL should use standard methods. Contributed (junping_du: rev 459a4833a90437a52787a41c2759a4b18cfe411c)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/URL.java
        • (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/records/TestURL.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10564 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10564/ ) YARN-5659 . getPathFromYarnURL should use standard methods. Contributed (junping_du: rev 459a4833a90437a52787a41c2759a4b18cfe411c) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/URL.java (add) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/test/java/org/apache/hadoop/yarn/api/records/TestURL.java
        Hide
        djp Junping Du added a comment -

        I have commit the latest patch (05) to trunk, branch-2 and branch-2.8. Thanks Sergey Shelukhin for patch contribution, also thanks Daniel Templeton and Wangda Tan for review and comments!

        Show
        djp Junping Du added a comment - I have commit the latest patch (05) to trunk, branch-2 and branch-2.8. Thanks Sergey Shelukhin for patch contribution, also thanks Daniel Templeton and Wangda Tan for review and comments!
        Hide
        templedf Daniel Templeton added a comment -

        LGTM.

        Show
        templedf Daniel Templeton added a comment - LGTM.
        Hide
        djp Junping Du added a comment -

        Latest patch LGTM. +1. Will commit it shortly if no further comments from others.

        Show
        djp Junping Du added a comment - Latest patch LGTM. +1. Will commit it shortly if no further comments from others.
        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 1 new or modified test files.
        +1 mvninstall 6m 52s trunk passed
        +1 compile 0m 23s trunk passed
        +1 checkstyle 0m 13s trunk passed
        +1 mvnsite 0m 27s trunk passed
        +1 mvneclipse 0m 13s trunk passed
        +1 findbugs 1m 3s trunk passed
        +1 javadoc 0m 16s trunk passed
        +1 mvninstall 0m 23s the patch passed
        +1 compile 0m 23s the patch passed
        +1 javac 0m 23s the patch passed
        +1 checkstyle 0m 11s the patch passed
        +1 mvnsite 0m 25s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 6s the patch passed
        +1 javadoc 0m 17s the patch passed
        +1 unit 0m 25s hadoop-yarn-api in the patch passed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        13m 59s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831674/YARN-5659.05.patch
        JIRA Issue YARN-5659
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 9636eb52862d 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 31f8da2
        Default Java 1.8.0_101
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13285/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13285/console
        Powered by Apache Yetus 0.3.0 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 1 new or modified test files. +1 mvninstall 6m 52s trunk passed +1 compile 0m 23s trunk passed +1 checkstyle 0m 13s trunk passed +1 mvnsite 0m 27s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 3s trunk passed +1 javadoc 0m 16s trunk passed +1 mvninstall 0m 23s the patch passed +1 compile 0m 23s the patch passed +1 javac 0m 23s the patch passed +1 checkstyle 0m 11s the patch passed +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 6s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 0m 25s hadoop-yarn-api in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 13m 59s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831674/YARN-5659.05.patch JIRA Issue YARN-5659 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9636eb52862d 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 31f8da2 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13285/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api Console output https://builds.apache.org/job/PreCommit-YARN-Build/13285/console Powered by Apache Yetus 0.3.0 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 14s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 7m 11s trunk passed
        +1 compile 0m 24s trunk passed
        +1 checkstyle 0m 13s trunk passed
        +1 mvnsite 0m 27s trunk passed
        +1 mvneclipse 0m 11s trunk passed
        +1 findbugs 1m 0s trunk passed
        +1 javadoc 0m 17s trunk passed
        +1 mvninstall 0m 21s the patch passed
        +1 compile 0m 21s the patch passed
        +1 javac 0m 21s the patch passed
        -1 checkstyle 0m 9s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api: The patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3)
        +1 mvnsite 0m 23s the patch passed
        +1 mvneclipse 0m 9s the patch passed
        -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix.
        +1 findbugs 1m 4s the patch passed
        +1 javadoc 0m 15s the patch passed
        +1 unit 0m 22s hadoop-yarn-api in the patch passed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        13m 53s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831672/YARN-5659.05.patch
        JIRA Issue YARN-5659
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 263651747df4 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 31f8da2
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13284/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13284/artifact/patchprocess/whitespace-eol.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13284/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13284/console
        Powered by Apache Yetus 0.3.0 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 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 11s trunk passed +1 compile 0m 24s trunk passed +1 checkstyle 0m 13s trunk passed +1 mvnsite 0m 27s trunk passed +1 mvneclipse 0m 11s trunk passed +1 findbugs 1m 0s trunk passed +1 javadoc 0m 17s trunk passed +1 mvninstall 0m 21s the patch passed +1 compile 0m 21s the patch passed +1 javac 0m 21s the patch passed -1 checkstyle 0m 9s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api: The patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3) +1 mvnsite 0m 23s the patch passed +1 mvneclipse 0m 9s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 findbugs 1m 4s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 0m 22s hadoop-yarn-api in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 13m 53s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831672/YARN-5659.05.patch JIRA Issue YARN-5659 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 263651747df4 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 31f8da2 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13284/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13284/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13284/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api Console output https://builds.apache.org/job/PreCommit-YARN-Build/13284/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        sershe Sergey Shelukhin added a comment -

        Added the annotations.
        Please feel free to change the patch wrt whitespace, annotations, method names, and other such stuff that is easier to change on commit than to have back-and-forth on the jira.

        Show
        sershe Sergey Shelukhin added a comment - Added the annotations. Please feel free to change the patch wrt whitespace, annotations, method names, and other such stuff that is easier to change on commit than to have back-and-forth on the jira.
        Hide
        hitesh Hitesh Shah added a comment - - edited

        Sergey Shelukhin Just annotate the functions that are only required by unit tests with @Private and @VisibleForTesting . In this case, the simplest approach would to use the above annotations for all the new functions that are added as part of this patch.

        Show
        hitesh Hitesh Shah added a comment - - edited Sergey Shelukhin Just annotate the functions that are only required by unit tests with @Private and @VisibleForTesting . In this case, the simplest approach would to use the above annotations for all the new functions that are added as part of this patch.
        Hide
        sershe Sergey Shelukhin added a comment -

        Hmm.. which one should I add? I am not very familiar with approach in YARN. Can someone add those on commit?

        Show
        sershe Sergey Shelukhin added a comment - Hmm.. which one should I add? I am not very familiar with approach in YARN. Can someone add those on commit?
        Hide
        leftnoteasy Wangda Tan added a comment -

        Oh got it, I looked at the patch in a wrong way.

        IIUC,
        public static URL fromURI(URI uri, Configuration conf)
        And
        public static URL fromPath(Path path, Configuration conf)
        Are only added for unit test?

        If so, to avoid adding unnecessary public API, could you make them to @private/@unstable/@visibleByTest?

        Show
        leftnoteasy Wangda Tan added a comment - Oh got it, I looked at the patch in a wrong way. IIUC, public static URL fromURI(URI uri, Configuration conf) And public static URL fromPath(Path path, Configuration conf) Are only added for unit test? If so, to avoid adding unnecessary public API, could you make them to @private/@unstable/@visibleByTest ?
        Hide
        sershe Sergey Shelukhin added a comment -

        This is an added overload, so it doesn't break the public API - the old one is still there; the config method is required to allow the test to override the RecordFactoryProvider configuration.

        Show
        sershe Sergey Shelukhin added a comment - This is an added overload, so it doesn't break the public API - the old one is still there; the config method is required to allow the test to override the RecordFactoryProvider configuration.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Sergey Shelukhin,

        I'm not quite sure why following is required:

        public static URL fromURI(URI uri, Configuration conf)
        

        Since this changes signature of a public/stable API, this is an incompatible change, could you comment about what is the issue caused by the original conf == null?

        Thanks,

        Show
        leftnoteasy Wangda Tan added a comment - Sergey Shelukhin , I'm not quite sure why following is required: public static URL fromURI(URI uri, Configuration conf) Since this changes signature of a public/stable API, this is an incompatible change, could you comment about what is the issue caused by the original conf == null? Thanks,
        Hide
        sershe Sergey Shelukhin added a comment -

        Daniel Templeton does the patch make sense now? only the whitespace was changed since the last iteration.
        Hitesh Shah fyi this one is ready

        Show
        sershe Sergey Shelukhin added a comment - Daniel Templeton does the patch make sense now? only the whitespace was changed since the last iteration. Hitesh Shah fyi this one is ready
        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 1 new or modified test files.
        +1 mvninstall 6m 37s trunk passed
        +1 compile 0m 24s trunk passed
        +1 checkstyle 0m 13s trunk passed
        +1 mvnsite 0m 25s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 1m 0s trunk passed
        +1 javadoc 0m 19s trunk passed
        +1 mvninstall 0m 22s the patch passed
        +1 compile 0m 20s the patch passed
        +1 javac 0m 20s the patch passed
        +1 checkstyle 0m 10s the patch passed
        +1 mvnsite 0m 25s the patch passed
        +1 mvneclipse 0m 9s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 12s the patch passed
        +1 javadoc 0m 14s the patch passed
        +1 unit 0m 22s hadoop-yarn-api in the patch passed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        13m 28s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829946/YARN-5659.04.patch
        JIRA Issue YARN-5659
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux ca76e6778e93 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 4fc632a
        Default Java 1.8.0_101
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13194/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13194/console
        Powered by Apache Yetus 0.3.0 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 1 new or modified test files. +1 mvninstall 6m 37s trunk passed +1 compile 0m 24s trunk passed +1 checkstyle 0m 13s trunk passed +1 mvnsite 0m 25s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 0s trunk passed +1 javadoc 0m 19s trunk passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 20s the patch passed +1 javac 0m 20s the patch passed +1 checkstyle 0m 10s the patch passed +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 12s the patch passed +1 javadoc 0m 14s the patch passed +1 unit 0m 22s hadoop-yarn-api in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 13m 28s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829946/YARN-5659.04.patch JIRA Issue YARN-5659 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ca76e6778e93 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 4fc632a Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13194/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api Console output https://builds.apache.org/job/PreCommit-YARN-Build/13194/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        sershe Sergey Shelukhin added a comment -

        The patch without a spurious change..

        Show
        sershe Sergey Shelukhin added a comment - The patch without a spurious change..
        Hide
        sershe Sergey Shelukhin added a comment -

        Apparently editing patches directly is not a good idea...

        Show
        sershe Sergey Shelukhin added a comment - Apparently editing patches directly is not a good idea...
        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 1 new or modified test files.
        +1 mvninstall 8m 12s trunk passed
        +1 compile 0m 27s trunk passed
        +1 checkstyle 0m 14s trunk passed
        +1 mvnsite 0m 28s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 1m 14s trunk passed
        +1 javadoc 0m 21s trunk passed
        -1 mvninstall 0m 28s hadoop-yarn-api in the patch failed.
        -1 compile 0m 28s hadoop-yarn-api in the patch failed.
        -1 javac 0m 28s hadoop-yarn-api in the patch failed.
        +1 checkstyle 0m 13s the patch passed
        -1 mvnsite 0m 26s hadoop-yarn-api in the patch failed.
        +1 mvneclipse 0m 12s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        -1 findbugs 0m 12s hadoop-yarn-api in the patch failed.
        +1 javadoc 0m 18s the patch passed
        -1 unit 0m 29s hadoop-yarn-api in the patch failed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        15m 10s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829926/YARN-5659.04.patch
        JIRA Issue YARN-5659
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux a5bd581463df 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 40acace
        Default Java 1.8.0_101
        findbugs v3.0.0
        mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        compile https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        javac https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13190/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13190/console
        Powered by Apache Yetus 0.3.0 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 1 new or modified test files. +1 mvninstall 8m 12s trunk passed +1 compile 0m 27s trunk passed +1 checkstyle 0m 14s trunk passed +1 mvnsite 0m 28s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 14s trunk passed +1 javadoc 0m 21s trunk passed -1 mvninstall 0m 28s hadoop-yarn-api in the patch failed. -1 compile 0m 28s hadoop-yarn-api in the patch failed. -1 javac 0m 28s hadoop-yarn-api in the patch failed. +1 checkstyle 0m 13s the patch passed -1 mvnsite 0m 26s hadoop-yarn-api in the patch failed. +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 0m 12s hadoop-yarn-api in the patch failed. +1 javadoc 0m 18s the patch passed -1 unit 0m 29s hadoop-yarn-api in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 15m 10s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829926/YARN-5659.04.patch JIRA Issue YARN-5659 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a5bd581463df 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 40acace Default Java 1.8.0_101 findbugs v3.0.0 mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt compile https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13190/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13190/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api Console output https://builds.apache.org/job/PreCommit-YARN-Build/13190/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        sershe Sergey Shelukhin added a comment -

        whitespace fixes...

        Show
        sershe Sergey Shelukhin added a comment - whitespace fixes...
        Hide
        templedf Daniel Templeton added a comment -

        The checkstyle and whitespace issue (both are the same) should be cleaned up.

        Show
        templedf Daniel Templeton added a comment - The checkstyle and whitespace issue (both are the same) should be cleaned up.
        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 1 new or modified test files.
        +1 mvninstall 8m 25s trunk passed
        +1 compile 0m 28s trunk passed
        +1 checkstyle 0m 14s trunk passed
        +1 mvnsite 0m 30s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 1m 11s trunk passed
        +1 javadoc 0m 18s trunk passed
        +1 mvninstall 0m 27s the patch passed
        +1 compile 0m 26s the patch passed
        +1 javac 0m 26s the patch passed
        -1 checkstyle 0m 11s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api: The patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3)
        +1 mvnsite 0m 26s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix.
        +1 findbugs 1m 15s the patch passed
        +1 javadoc 0m 15s the patch passed
        +1 unit 0m 26s hadoop-yarn-api in the patch passed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        16m 2s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829692/YARN-5659.03.patch
        JIRA Issue YARN-5659
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 078c521328ff 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 964e546
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13181/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13181/artifact/patchprocess/whitespace-eol.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13181/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13181/console
        Powered by Apache Yetus 0.3.0 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 1 new or modified test files. +1 mvninstall 8m 25s trunk passed +1 compile 0m 28s trunk passed +1 checkstyle 0m 14s trunk passed +1 mvnsite 0m 30s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 11s trunk passed +1 javadoc 0m 18s trunk passed +1 mvninstall 0m 27s the patch passed +1 compile 0m 26s the patch passed +1 javac 0m 26s the patch passed -1 checkstyle 0m 11s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api: The patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3) +1 mvnsite 0m 26s the patch passed +1 mvneclipse 0m 10s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 findbugs 1m 15s the patch passed +1 javadoc 0m 15s the patch passed +1 unit 0m 26s hadoop-yarn-api in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 16m 2s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829692/YARN-5659.03.patch JIRA Issue YARN-5659 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 078c521328ff 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 964e546 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13181/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13181/artifact/patchprocess/whitespace-eol.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13181/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api Console output https://builds.apache.org/job/PreCommit-YARN-Build/13181/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment -

        Ah, sorry. I assumed we were looking for something like "://foo" for that test. Do the tests cover all the issues that you found in the previous implementation? Would it be useful to do any negative testing?

        Show
        templedf Daniel Templeton added a comment - Ah, sorry. I assumed we were looking for something like "://foo" for that test. Do the tests cover all the issues that you found in the previous implementation? Would it be useful to do any negative testing?
        Hide
        sershe Sergey Shelukhin added a comment -

        Daniel Templeton the test already has paths without a schema

        Show
        sershe Sergey Shelukhin added a comment - Daniel Templeton the test already has paths without a schema
        Hide
        sershe Sergey Shelukhin added a comment -

        Fixing the warnings

        Show
        sershe Sergey Shelukhin added a comment - Fixing the warnings
        Hide
        templedf Daniel Templeton added a comment -

        Thanks for the updated patch. The tests look good. Think you can add tests for the bugs you saw in the previous implementation, e.g. empty schema?

        Show
        templedf Daniel Templeton added a comment - Thanks for the updated patch. The tests look good. Think you can add tests for the bugs you saw in the previous implementation, e.g. empty schema?
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 18s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 7m 13s trunk passed
        +1 compile 0m 23s trunk passed
        +1 checkstyle 0m 12s trunk passed
        +1 mvnsite 0m 26s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 1m 0s trunk passed
        +1 javadoc 0m 17s trunk passed
        +1 mvninstall 0m 22s the patch passed
        +1 compile 0m 20s the patch passed
        +1 javac 0m 20s the patch passed
        -1 checkstyle 0m 10s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api: The patch generated 7 new + 3 unchanged - 0 fixed = 10 total (was 3)
        +1 mvnsite 0m 23s the patch passed
        +1 mvneclipse 0m 9s the patch passed
        -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix.
        -1 findbugs 1m 6s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
        +1 javadoc 0m 14s the patch passed
        +1 unit 0m 22s hadoop-yarn-api in the patch passed.
        +1 asflicense 0m 18s The patch does not generate ASF License warnings.
        14m 3s



        Reason Tests
        FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api
          There is an apparent infinite recursive loop in org.apache.hadoop.yarn.api.records.URL.fromURI(URI) At URL.java:recursive loop in org.apache.hadoop.yarn.api.records.URL.fromURI(URI) At URL.java:[line 158]



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829677/YARN-5659.02.patch
        JIRA Issue YARN-5659
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 1512cafb93a4 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 964e546
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13180/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13180/artifact/patchprocess/whitespace-eol.txt
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/13180/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.html
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13180/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13180/console
        Powered by Apache Yetus 0.3.0 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 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 13s trunk passed +1 compile 0m 23s trunk passed +1 checkstyle 0m 12s trunk passed +1 mvnsite 0m 26s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 0s trunk passed +1 javadoc 0m 17s trunk passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 20s the patch passed +1 javac 0m 20s the patch passed -1 checkstyle 0m 10s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api: The patch generated 7 new + 3 unchanged - 0 fixed = 10 total (was 3) +1 mvnsite 0m 23s the patch passed +1 mvneclipse 0m 9s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix. -1 findbugs 1m 6s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 14s the patch passed +1 unit 0m 22s hadoop-yarn-api in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 14m 3s Reason Tests FindBugs module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api   There is an apparent infinite recursive loop in org.apache.hadoop.yarn.api.records.URL.fromURI(URI) At URL.java:recursive loop in org.apache.hadoop.yarn.api.records.URL.fromURI(URI) At URL.java: [line 158] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829677/YARN-5659.02.patch JIRA Issue YARN-5659 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1512cafb93a4 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 964e546 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13180/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt whitespace https://builds.apache.org/job/PreCommit-YARN-Build/13180/artifact/patchprocess/whitespace-eol.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/13180/artifact/patchprocess/new-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.html Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13180/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api Console output https://builds.apache.org/job/PreCommit-YARN-Build/13180/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        sershe Sergey Shelukhin added a comment -

        Added the test. The test only tests the static methods, so it uses a fake URL class to sidestep factory visibility issues.

        Show
        sershe Sergey Shelukhin added a comment - Added the test. The test only tests the static methods, so it uses a fake URL class to sidestep factory visibility issues.
        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 doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        +1 mvninstall 7m 1s trunk passed
        +1 compile 0m 26s trunk passed
        +1 checkstyle 0m 13s trunk passed
        +1 mvnsite 0m 27s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 1m 5s trunk passed
        +1 javadoc 0m 18s trunk passed
        -1 mvninstall 0m 19s hadoop-yarn-api in the patch failed.
        -1 compile 0m 20s hadoop-yarn-api in the patch failed.
        -1 javac 0m 20s hadoop-yarn-api in the patch failed.
        +1 checkstyle 0m 10s the patch passed
        -1 mvnsite 0m 19s hadoop-yarn-api in the patch failed.
        +1 mvneclipse 0m 10s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        -1 findbugs 0m 12s hadoop-yarn-api in the patch failed.
        +1 javadoc 0m 14s the patch passed
        -1 unit 0m 20s hadoop-yarn-api in the patch failed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        12m 53s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829487/YARN-5659.01.patch
        JIRA Issue YARN-5659
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 54b22a3583bc 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 964e546
        Default Java 1.8.0_101
        findbugs v3.0.0
        mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        compile https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        javac https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        findbugs https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13171/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13171/console
        Powered by Apache Yetus 0.3.0 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 doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 7m 1s trunk passed +1 compile 0m 26s trunk passed +1 checkstyle 0m 13s trunk passed +1 mvnsite 0m 27s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 5s trunk passed +1 javadoc 0m 18s trunk passed -1 mvninstall 0m 19s hadoop-yarn-api in the patch failed. -1 compile 0m 20s hadoop-yarn-api in the patch failed. -1 javac 0m 20s hadoop-yarn-api in the patch failed. +1 checkstyle 0m 10s the patch passed -1 mvnsite 0m 19s hadoop-yarn-api in the patch failed. +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 0m 12s hadoop-yarn-api in the patch failed. +1 javadoc 0m 14s the patch passed -1 unit 0m 20s hadoop-yarn-api in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 12m 53s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829487/YARN-5659.01.patch JIRA Issue YARN-5659 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 54b22a3583bc 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 964e546 Default Java 1.8.0_101 findbugs v3.0.0 mvninstall https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt compile https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt javac https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-compile-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt mvnsite https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-mvnsite-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt findbugs https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-findbugs-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13171/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13171/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api Console output https://builds.apache.org/job/PreCommit-YARN-Build/13171/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        sershe Sergey Shelukhin added a comment -

        The patch for trunk, where this method has been moved. What's the difference between trunk and master? Do both need to be fixed?

        Show
        sershe Sergey Shelukhin added a comment - The patch for trunk, where this method has been moved. What's the difference between trunk and master? Do both need to be fixed?
        Hide
        djp Junping Du added a comment -

        Thanks Daniel Templeton for review and comments! Patch looks OK to me but I agree with Daniel that we should add a simple unit test for a normal URL and URL without scheme (that you mentioned a bug exists so far).
        +1 when adding UT there.

        Show
        djp Junping Du added a comment - Thanks Daniel Templeton for review and comments! Patch looks OK to me but I agree with Daniel that we should add a simple unit test for a normal URL and URL without scheme (that you mentioned a bug exists so far). +1 when adding UT there.
        Hide
        hitesh Hitesh Shah added a comment -
        Show
        hitesh Hitesh Shah added a comment - \cc Wangda Tan Varun Vasudev Junping Du
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        -1 patch 0m 4s YARN-5659 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



        Subsystem Report/Notes
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829444/YARN-5659.patch
        JIRA Issue YARN-5659
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13168/console
        Powered by Apache Yetus 0.3.0 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 0s Docker mode activated. -1 patch 0m 4s YARN-5659 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829444/YARN-5659.patch JIRA Issue YARN-5659 Console output https://builds.apache.org/job/PreCommit-YARN-Build/13168/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment - - edited

        Looks right to me. It would be really nice to add some unit tests to make sure the change isn't breaking anything.

        Show
        templedf Daniel Templeton added a comment - - edited Looks right to me. It would be really nice to add some unit tests to make sure the change isn't breaking anything.
        Hide
        sershe Sergey Shelukhin added a comment -

        The patch. Based on the reversal of getYarnURLFromPath/URI.
        normalize is also unneeded since Path already does that.

        Hitesh Shah can you take a look?

        Show
        sershe Sergey Shelukhin added a comment - The patch. Based on the reversal of getYarnURLFromPath/URI. normalize is also unneeded since Path already does that. Hitesh Shah can you take a look?

          People

          • Assignee:
            sershe Sergey Shelukhin
            Reporter:
            sershe Sergey Shelukhin
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development