Hadoop Common
  1. Hadoop Common
  2. HADOOP-6531

add FileUtil.fullyDeleteContents(dir) api to delete contents of a directory

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: fs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Added an api FileUtil.fullyDeleteContents(String dir) to delete contents of the directory.

      Description

      Add FileUtil.fullyDeleteContents(dir) api to delete contents of a directory, not directory itself. This will be useful if we want to clear the contents of cwd.

      1. patch-6531.txt
        4 kB
        Amareshwari Sriramadasu
      2. patch-6531-1.txt
        4 kB
        Amareshwari Sriramadasu
      3. patch-6531-2.txt
        4 kB
        Amareshwari Sriramadasu
      4. patch-6531-3.txt
        6 kB
        Amareshwari Sriramadasu

        Issue Links

          Activity

          Amareshwari Sriramadasu created issue -
          Hide
          Amareshwari Sriramadasu added a comment -

          Patch adding the api.

          Show
          Amareshwari Sriramadasu added a comment - Patch adding the api.
          Amareshwari Sriramadasu made changes -
          Field Original Value New Value
          Attachment patch-6531.txt [ 12434506 ]
          Amareshwari Sriramadasu made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Amareshwari Sriramadasu made changes -
          Link This issue blocks MAPREDUCE-1435 [ MAPREDUCE-1435 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12434506/patch-6531.txt
          against trunk revision 904975.

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

          +1 tests included. The patch appears to include 3 new or modified tests.

          +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 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 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/312/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/312/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/312/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/312/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/12434506/patch-6531.txt against trunk revision 904975. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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 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 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/312/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/312/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/312/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/312/console This message is automatically generated.
          Amareshwari Sriramadasu made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Amareshwari Sriramadasu added a comment -

          Changed the unit test to use Junit v4 style

          Show
          Amareshwari Sriramadasu added a comment - Changed the unit test to use Junit v4 style
          Amareshwari Sriramadasu made changes -
          Attachment patch-6531-1.txt [ 12434508 ]
          Amareshwari Sriramadasu made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12434508/patch-6531-1.txt
          against trunk revision 904975.

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

          +1 tests included. The patch appears to include 3 new or modified tests.

          +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 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 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/313/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/313/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/313/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/313/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/12434508/patch-6531-1.txt against trunk revision 904975. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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 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 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/313/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/313/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/313/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/313/console This message is automatically generated.
          Hide
          Ravi Gummadi added a comment -

          We are not checking the return value of fullyDeleteContents() in fullyDelete() method. But if fullyDeleteContents() returns false, next line "dir.delete()" will be executed extra for all the directories in the heirarchy that we traversed. Though it is harmless with this existing patch, it looks better to have the check here and return false if fullyDeleteContents() returns false.

          Minor nit in testcase: Last 3 assertions in both testcases in TestFileUtil.java are same and can be moved to a separate method (say validateTmpDir() ) to avoid code-duplication.

          Show
          Ravi Gummadi added a comment - We are not checking the return value of fullyDeleteContents() in fullyDelete() method. But if fullyDeleteContents() returns false, next line "dir.delete()" will be executed extra for all the directories in the heirarchy that we traversed. Though it is harmless with this existing patch, it looks better to have the check here and return false if fullyDeleteContents() returns false. Minor nit in testcase: Last 3 assertions in both testcases in TestFileUtil.java are same and can be moved to a separate method (say validateTmpDir() ) to avoid code-duplication.
          Amareshwari Sriramadasu made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Amareshwari Sriramadasu added a comment -

          Thanks for the quick review Ravi.
          Patch with comments incorporated.

          Show
          Amareshwari Sriramadasu added a comment - Thanks for the quick review Ravi. Patch with comments incorporated.
          Amareshwari Sriramadasu made changes -
          Attachment patch-6531-2.txt [ 12434518 ]
          Amareshwari Sriramadasu made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12434518/patch-6531-2.txt
          against trunk revision 904975.

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

          +1 tests included. The patch appears to include 3 new or modified tests.

          +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 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 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/314/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/314/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/314/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/314/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/12434518/patch-6531-2.txt against trunk revision 904975. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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 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 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/314/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/314/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/314/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/314/console This message is automatically generated.
          Hide
          Hemanth Yamijala added a comment -

          I would improve the test cases just a little bit:

          • Please check for return values of FileUtil.fullyDeleteContents and FileUtil.fullyDelete in test case.
          • Also add a failure test case for both fullyDelete and fullyDeleteContents. We can create a new directory for which we turn off write permissions. Then fullyDelete and fullyDeleteContents should both fail and we should get a false return value that can be checked.

          Code changes seem fine to me.

          Show
          Hemanth Yamijala added a comment - I would improve the test cases just a little bit: Please check for return values of FileUtil.fullyDeleteContents and FileUtil.fullyDelete in test case. Also add a failure test case for both fullyDelete and fullyDeleteContents. We can create a new directory for which we turn off write permissions. Then fullyDelete and fullyDeleteContents should both fail and we should get a false return value that can be checked. Code changes seem fine to me.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Can we also fix the javadoc of each of the fullyDelete*() methods to describe their behavior with directories, files and symlinks?

          Show
          Vinod Kumar Vavilapalli added a comment - Can we also fix the javadoc of each of the fullyDelete*() methods to describe their behavior with directories, files and symlinks?
          Amareshwari Sriramadasu made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Amareshwari Sriramadasu added a comment -

          Patch modifying the testcase as suggested.

          Can we also fix the javadoc of each of the fullyDelete*() methods to describe their behavior with directories, files and symlinks?

          Created HADOOP-6536 for this.

          Show
          Amareshwari Sriramadasu added a comment - Patch modifying the testcase as suggested. Can we also fix the javadoc of each of the fullyDelete*() methods to describe their behavior with directories, files and symlinks? Created HADOOP-6536 for this.
          Amareshwari Sriramadasu made changes -
          Attachment patch-6531-3.txt [ 12434646 ]
          Amareshwari Sriramadasu made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12434646/patch-6531-3.txt
          against trunk revision 905860.

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

          +1 tests included. The patch appears to include 3 new or modified tests.

          +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 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 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/322/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/322/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/322/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/322/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/12434646/patch-6531-3.txt against trunk revision 905860. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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 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 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/322/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/322/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/322/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/322/console This message is automatically generated.
          Hide
          Hemanth Yamijala added a comment -

          Seems fine to me. +1.

          I think HADOOP-6536 will result in a fix to code, not just to documentation. Hence, I think it is OK to address it in a separate JIRA.

          Show
          Hemanth Yamijala added a comment - Seems fine to me. +1. I think HADOOP-6536 will result in a fix to code, not just to documentation. Hence, I think it is OK to address it in a separate JIRA.
          Hide
          Hemanth Yamijala added a comment -

          I just committed this to trunk. Thanks, Amareshwari !

          Can you please update the release note with the relevant API added.

          Show
          Hemanth Yamijala added a comment - I just committed this to trunk. Thanks, Amareshwari ! Can you please update the release note with the relevant API added.
          Hemanth Yamijala made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags [Reviewed]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #156 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/156/)
          . Enhance FileUtil with an API to delete all contents of a directory. Contributed by Amareshwari Sriramadasu.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #156 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/156/ ) . Enhance FileUtil with an API to delete all contents of a directory. Contributed by Amareshwari Sriramadasu.
          Amareshwari Sriramadasu made changes -
          Release Note Added an api FileUtil.fullyDeleteContents(String dir) to delete contents of the directory.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #242 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/242/)
          . Enhance FileUtil with an API to delete all contents of a directory. Contributed by Amareshwari Sriramadasu.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #242 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/242/ ) . Enhance FileUtil with an API to delete all contents of a directory. Contributed by Amareshwari Sriramadasu.
          Tom White made changes -
          Fix Version/s 0.21.0 [ 12313563 ]
          Fix Version/s 0.22.0 [ 12314296 ]
          Tom White made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Patch Available Patch Available Open Open
          22h 33m 3 Amareshwari Sriramadasu 03/Feb/10 05:54
          Open Open Patch Available Patch Available
          1h 32m 4 Amareshwari Sriramadasu 03/Feb/10 05:58
          Patch Available Patch Available Resolved Resolved
          13h 7m 1 Hemanth Yamijala 03/Feb/10 19:05
          Resolved Resolved Closed Closed
          202d 1h 35m 1 Tom White 24/Aug/10 21:41

            People

            • Assignee:
              Amareshwari Sriramadasu
              Reporter:
              Amareshwari Sriramadasu
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development