Hadoop Common
  1. Hadoop Common
  2. HADOOP-7447

Add a warning message for FsShell -getmerge when the src path is no a directory

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Duplicate
    • Affects Version/s: 0.20.3
    • Fix Version/s: None
    • Component/s: fs
    • Labels:
      None

      Description

      While the <src> specified for FsShell -getmerge is not a directory,the command does nothing and there's no any message explaining the issue.
      Furthermore,the exitCode is zero.

      $ hdfs dfs -getmerge /user/hadoop/testfile /work/tmp/testfile
      $ echo $?
      0
      $ ls /work/tmp/testfile
      ls: cannot access /work/tmp/testfile: No such file or directory

      1. HADOOP-7447
        0.6 kB
        XieXianshan
      2. HADOOP-7447.patch
        0.8 kB
        XieXianshan
      3. HADOOP-7447-merge.patch
        1.0 kB
        XieXianshan
      4. HADOOP-7447-v0.3.patch
        1 kB
        XieXianshan
      5. HADOOP-7447-v0.4.patch
        2 kB
        XieXianshan

        Issue Links

          Activity

          Hide
          XieXianshan added a comment -

          Fixed it.

          Show
          XieXianshan added a comment - Fixed it.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12485387/HADOOP-7447
          against trunk revision 1143219.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/699//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/699//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/699//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12485387/HADOOP-7447 against trunk revision 1143219. +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/699//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/699//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/699//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          To be consistent with the rest of the code, the main change should be:

          if (!src.stat.isDirectory()) {
            throw new PathIsNotDirectoryException(src.toString());
          }
          

          Since copyMerge apparently returns null on various failures, you should check for that too (add newlines as you see fit):

          if (FileUtil.copyMerge(src.fs, src.path, dst.fs, dst.path, false, getConf(), delimiter) == null) {
            throw new PathIOException(src.toString());
          }
          

          Please add some tests to TestHDFSCLI to cover the cases you've fixed. Thanks!

          Show
          Daryn Sharp added a comment - To be consistent with the rest of the code, the main change should be: if (!src.stat.isDirectory()) { throw new PathIsNotDirectoryException(src.toString()); } Since copyMerge apparently returns null on various failures, you should check for that too (add newlines as you see fit): if (FileUtil.copyMerge(src.fs, src.path, dst.fs, dst.path, false , getConf(), delimiter) == null ) { throw new PathIOException(src.toString()); } Please add some tests to TestHDFSCLI to cover the cases you've fixed. Thanks!
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12486310/HADOOP-7447.patch
          against trunk revision 1145839.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/724//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/724//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/724//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12486310/HADOOP-7447.patch against trunk revision 1145839. +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/724//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/724//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/724//console This message is automatically generated.
          Hide
          XieXianshan added a comment -

          Thanks for your comment.I reuploaded the patch.

          Show
          XieXianshan added a comment - Thanks for your comment.I reuploaded the patch.
          Hide
          Daryn Sharp added a comment -

          Did you mean to drop the isDirectory() check in the second patch? I'd like to see that in the patch too.

          Show
          Daryn Sharp added a comment - Did you mean to drop the isDirectory() check in the second patch? I'd like to see that in the patch too.
          Hide
          XieXianshan added a comment -

          I merged the two patches.

          Show
          XieXianshan added a comment - I merged the two patches.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12487797/HADOOP-7447-merge.patch
          against trunk revision 1150987.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/765//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/765//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/765//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12487797/HADOOP-7447-merge.patch against trunk revision 1150987. +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/765//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/765//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/765//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          Just a few small issues. Re-stating the path isn't necessary, and the exception is inconsistent with the rest of the commands.

          if (!src.fs.getFileStatus(src.path).isDirectory()) {
            throw new IOException(src.path.toString() + " is not a directory");
          }
          

          As formally cited, it should be:

          if (!src.stat.isDirectory()) {
            throw new PathIsNotDirectoryException(src.toString());
          }
          

          Otherwise, it's good to go! Good job.

          Show
          Daryn Sharp added a comment - Just a few small issues. Re-stating the path isn't necessary, and the exception is inconsistent with the rest of the commands. if (!src.fs.getFileStatus(src.path).isDirectory()) { throw new IOException(src.path.toString() + " is not a directory" ); } As formally cited, it should be: if (!src.stat.isDirectory()) { throw new PathIsNotDirectoryException(src.toString()); } Otherwise, it's good to go! Good job.
          Hide
          XieXianshan added a comment -

          Reupdated.Thanks Daryn.You are always so kind.

          Show
          XieXianshan added a comment - Reupdated.Thanks Daryn.You are always so kind.
          Hide
          Daryn Sharp added a comment -

          +1 but I think there's an issue with the linked hdfs jira expecting the wrong output, so this shouldn't be committed until that issue is resolved. Great work!

          Show
          Daryn Sharp added a comment - +1 but I think there's an issue with the linked hdfs jira expecting the wrong output, so this shouldn't be committed until that issue is resolved. Great work!
          Hide
          Daryn Sharp added a comment -

          +1 again. The test jira has been correctly updated. Great work!

          Show
          Daryn Sharp added a comment - +1 again. The test jira has been correctly updated. Great work!
          Hide
          XieXianshan added a comment -

          Updated the patch with new directory structure.

          Show
          XieXianshan added a comment - Updated the patch with new directory structure.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12493133/HADOOP-7447-v0.4.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth-examples.html
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12493133/HADOOP-7447-v0.4.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-common-project. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-auth-examples.html Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/139//console This message is automatically generated.
          Hide
          XieXianshan added a comment -

          Can someone please review and commit this patch?

          Show
          XieXianshan added a comment - Can someone please review and commit this patch?

            People

            • Assignee:
              XieXianshan
              Reporter:
              XieXianshan
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development