Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.15.0
    • Fix Version/s: 0.15.0
    • Component/s: io
    • Labels:
      None

      Description

      In the current situation, FileUtil class includes both file related and io related functionality. This issue intends to separate the two.

      1. IOUtils_v1.0.patch
        25 kB
        Enis Soztutar
      2. IOUtils_v1.1.patch
        24 kB
        Enis Soztutar
      3. IOUtils_v1.2.patch
        25 kB
        Enis Soztutar
      4. IOUtils_v1.3.patch
        25 kB
        Enis Soztutar
      5. IOUtils_v1.4.patch
        21 kB
        Raghu Angadi
      6. IOUtils_v1.4.patch
        26 kB
        Raghu Angadi
      7. IOUtils_v1.5.patch
        26 kB
        Enis Soztutar
      8. IOUtils_v1.6.patch
        26 kB
        Enis Soztutar

        Issue Links

          Activity

          Hide
          Enis Soztutar added a comment -

          This patch adds a new class IOUtils, refactors some functions from FileUtil and FsShell, deprecates puclic methods, and updates the referances. I have included removed many @Override annotations added by Eclipse, which i find quite handy.

          Show
          Enis Soztutar added a comment - This patch adds a new class IOUtils , refactors some functions from FileUtil and FsShell , deprecates puclic methods, and updates the referances. I have included removed many @Override annotations added by Eclipse, which i find quite handy.
          Hide
          Hadoop QA added a comment -

          -1, new javadoc warnings

          The javadoc tool appears to have generated warning messages when testing the latest attachment http://issues.apache.org/jira/secure/attachment/12362608/IOUtils_v1.0.patch against trunk revision r559623.

          Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/469/testReport/
          Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/469/console

          Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.

          Show
          Hadoop QA added a comment - -1, new javadoc warnings The javadoc tool appears to have generated warning messages when testing the latest attachment http://issues.apache.org/jira/secure/attachment/12362608/IOUtils_v1.0.patch against trunk revision r559623. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/469/testReport/ Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/469/console Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.
          Hide
          Raghu Angadi added a comment -

          You can remove FileUtil versions of readFully() etc, instead of deprecating them. They were introduced recently (HADOOP-1134) and not widely used yet.

          Show
          Raghu Angadi added a comment - You can remove FileUtil versions of readFully() etc, instead of deprecating them. They were introduced recently ( HADOOP-1134 ) and not widely used yet.
          Hide
          Enis Soztutar added a comment -

          Removed FileUtil methods which had been moved to IOUtils. Fixes the javadoc warning.

          Show
          Enis Soztutar added a comment - Removed FileUtil methods which had been moved to IOUtils . Fixes the javadoc warning.
          Hide
          Hadoop QA added a comment -

          -1, build or testing failed

          2 attempts failed to build and test the latest attachment http://issues.apache.org/jira/secure/attachment/12362626/IOUtils_v1.1.patch against trunk revision r559819.

          Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/470/testReport/
          Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/470/console

          Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.

          Show
          Hadoop QA added a comment - -1, build or testing failed 2 attempts failed to build and test the latest attachment http://issues.apache.org/jira/secure/attachment/12362626/IOUtils_v1.1.patch against trunk revision r559819. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/470/testReport/ Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/470/console Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.
          Hide
          Enis Soztutar added a comment -

          Added fix for recently committed TestDataTransferProtocol.

          Show
          Enis Soztutar added a comment - Added fix for recently committed TestDataTransferProtocol .
          Show
          Hadoop QA added a comment - +1 http://issues.apache.org/jira/secure/attachment/12362629/IOUtils_v1.2.patch applied and successfully tested against trunk revision r559819. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/472/testReport/ Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/472/console
          Hide
          Raghu Angadi added a comment -

          Patch looks good. My only reservation is copyBytes() which takes conf.. Why not just require a buffer size. This is a utility function that is expected to be used in variety of places/contexts. conf is not always available. What do you think?

          Show
          Raghu Angadi added a comment - Patch looks good. My only reservation is copyBytes() which takes conf.. Why not just require a buffer size. This is a utility function that is expected to be used in variety of places/contexts. conf is not always available. What do you think?
          Hide
          Enis Soztutar added a comment -

          There is already an overloaded version which does not take conf, but buffSize.

          public static void copyBytes(InputStream in, OutputStream out, int buffSize, boolean close)
          
          Show
          Enis Soztutar added a comment - There is already an overloaded version which does not take conf, but buffSize. public static void copyBytes(InputStream in, OutputStream out, int buffSize, boolean close)
          Hide
          Enis Soztutar added a comment -

          Updated the patch for the current trunk.

          Show
          Enis Soztutar added a comment - Updated the patch for the current trunk.
          Show
          Hadoop QA added a comment - +1 http://issues.apache.org/jira/secure/attachment/12363250/IOUtils_v1.3.patch applied and successfully tested against trunk revision r562608. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/518/testReport/ Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/518/console
          Hide
          Raghu Angadi added a comment -

          Enis,

          HADOOP-1649 adds 'skipFully()' in the same wrong place. We need to update this jira with that. I can update the patch. Let me know. For now I am removing 'Patch Available' state.

          Show
          Raghu Angadi added a comment - Enis, HADOOP-1649 adds 'skipFully()' in the same wrong place. We need to update this jira with that. I can update the patch. Let me know. For now I am removing 'Patch Available' state.
          Hide
          Raghu Angadi added a comment -

          Updated the patch with latest trunk and added skipFully() to IOUtils.

          Show
          Raghu Angadi added a comment - Updated the patch with latest trunk and added skipFully() to IOUtils.
          Hide
          Raghu Angadi added a comment -

          add missing IOUtils.java.

          Show
          Raghu Angadi added a comment - add missing IOUtils.java.
          Hide
          Enis Soztutar added a comment -

          Retriggering the hudson build, since last time it was not. Local tests succeeds.

          Show
          Enis Soztutar added a comment - Retriggering the hudson build, since last time it was not. Local tests succeeds.
          Hide
          Hadoop QA added a comment -

          -1, could not apply patch.

          The patch command could not apply the latest attachment http://issues.apache.org/jira/secure/attachment/12363631/IOUtils_v1.4.patch as a patch to trunk revision r565995.

          Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/557/console

          Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.

          Show
          Hadoop QA added a comment - -1, could not apply patch. The patch command could not apply the latest attachment http://issues.apache.org/jira/secure/attachment/12363631/IOUtils_v1.4.patch as a patch to trunk revision r565995. Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/557/console Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.
          Hide
          Enis Soztutar added a comment -

          Updated the patch for the current trunk.

          Show
          Enis Soztutar added a comment - Updated the patch for the current trunk.
          Hide
          Hadoop QA added a comment -

          -1, build or testing failed

          2 attempts failed to build and test the latest attachment http://issues.apache.org/jira/secure/attachment/12363840/IOUtils_v1.5.patch against trunk revision r565995.

          Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/558/testReport/
          Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/558/console

          Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.

          Show
          Hadoop QA added a comment - -1, build or testing failed 2 attempts failed to build and test the latest attachment http://issues.apache.org/jira/secure/attachment/12363840/IOUtils_v1.5.patch against trunk revision r565995. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/558/testReport/ Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/558/console Please note that this message is automatically generated and may represent a problem with the automation system and not the patch.
          Hide
          Enis Soztutar added a comment -

          Retriggering the build, yet again, after HADOOP-1717 is fixed.

          Show
          Enis Soztutar added a comment - Retriggering the build, yet again, after HADOOP-1717 is fixed.
          Show
          Hadoop QA added a comment - +1 http://issues.apache.org/jira/secure/attachment/12363840/IOUtils_v1.5.patch applied and successfully tested against trunk revision r566467. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/562/testReport/ Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/562/console
          Hide
          Doug Cutting added a comment -

          Sorry, but this patch no longer applies to trunk. Can you please update it?

          Also, shouldn't the public methods in FileUtil that are removed instead be deprecated in 0.15? Then we should file an issue to remove them in 0.16 or later.

          Show
          Doug Cutting added a comment - Sorry, but this patch no longer applies to trunk. Can you please update it? Also, shouldn't the public methods in FileUtil that are removed instead be deprecated in 0.15? Then we should file an issue to remove them in 0.16 or later.
          Hide
          Enis Soztutar added a comment -

          > shouldn't the public methods in FileUtil that are removed instead be deprecated in 0.15
          Version 1.0 of the patch deprecated the public methods in FileUtil, but Raghu has suggested to remove the functions since they are not widely used (see above). I agree with Raghu in this, especially relying on the heuristic that FileUtil#copyBytes() or similar functions are quite hadoop internal (although public). However I will just deprecate the methods if you insist. smile

          Show
          Enis Soztutar added a comment - > shouldn't the public methods in FileUtil that are removed instead be deprecated in 0.15 Version 1.0 of the patch deprecated the public methods in FileUtil , but Raghu has suggested to remove the functions since they are not widely used (see above). I agree with Raghu in this, especially relying on the heuristic that FileUtil#copyBytes() or similar functions are quite hadoop internal (although public). However I will just deprecate the methods if you insist. smile
          Hide
          Enis Soztutar added a comment -

          The latest patch updated to trunk.

          I will not trigger hudson build until we decide on deprecation/removal of public methods that are moved to IOUtils.

          Show
          Enis Soztutar added a comment - The latest patch updated to trunk. I will not trigger hudson build until we decide on deprecation/removal of public methods that are moved to IOUtils.
          Hide
          Raghu Angadi added a comment -

          Since most of these were added only recently in 0.14 (as part of Block CRCs), I prefer to remove them. I am not sure about copyBytes() but rest of these are not used outside dfs package.

          Show
          Raghu Angadi added a comment - Since most of these were added only recently in 0.14 (as part of Block CRCs), I prefer to remove them. I am not sure about copyBytes() but rest of these are not used outside dfs package.
          Hide
          Doug Cutting added a comment -

          Okay, I'll agree to bend the rules a bit, and remove these public methods that were in a release, since we don't think anyone will have used them.

          Show
          Doug Cutting added a comment - Okay, I'll agree to bend the rules a bit, and remove these public methods that were in a release, since we don't think anyone will have used them.
          Show
          Hadoop QA added a comment - +1 http://issues.apache.org/jira/secure/attachment/12364290/IOUtils_v1.6.patch applied and successfully tested against trunk revision r568700. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/595/testReport/ Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/595/console
          Hide
          Doug Cutting added a comment -

          I just committed this. Thanks, Enis!

          Show
          Doug Cutting added a comment - I just committed this. Thanks, Enis!

            People

            • Assignee:
              Enis Soztutar
              Reporter:
              Enis Soztutar
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development