Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.3.0
    • Component/s: Core
    • Labels:
      None

      Description

      The FileSystem class is overloaded and has methods that are not well supported. I suggest to do the following cleanups:

      • Pull the safety net into a separate class
      • Use the WriteMode to indicate overwriting behavior. Right now, the FileSystem class defines that enum and never uses it. It feels weird.
      • Remove the create(path, overwrite, blocksize, reolication, ...) method, which is not really supported across file system implementations. For HDFS, behavior should be set via the configuration anyways.

      All changes have to be made in a non-API-breaking fashion.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user StephanEwen opened a pull request:

          https://github.com/apache/flink/pull/3328

          FLINK-5812 [core] Cleanups in the FileSystem class

          The FileSystem class is overloaded and has methods that are not well supported.

          This pull request does the following cleanups:

          Commit 1:

          • Pull the safety net into a separate class
          • Extend the docs of the safety net.

          Commit 2:

          • Use the `WriteMode` to indicate overwriting behavior. Right now, the `FileSystem` class defines that enum and never uses it. It feels weird.
          • Remove the method `FsDataOutputStream create(path, overwrite, blocksize, replication, ...)`, which is not really supported across file system implementations. For HDFS, the behavior should be set via the configuration anyways.

          All changes have to be made in a non-API-breaking fashion.

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

          $ git pull https://github.com/StephanEwen/incubator-flink fs_cleanup

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

          https://github.com/apache/flink/pull/3328.patch

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

          This closes #3328


          commit 6c86f901427d4bb077f27c84c90abeafec019449
          Author: Stephan Ewen <sewen@apache.org>
          Date: 2017-02-15T16:10:53Z

          FLINK-5812 [core] Cleanups in FileSystem (round 1)

          • This makes the FileSystem use the 'WriteMode' (otherwise it was an unused enumeration)
          • Extends comments
          • Deprecate the method that controls the replication factor and block size

          commit 83578e2b8e2e1ca03b14e8d47f7a122aa07fd517
          Author: Stephan Ewen <sewen@apache.org>
          Date: 2017-02-15T16:58:37Z

          FLINK-5812 [core] Cleanups in FileSystem (round 2)

          Move the FileSystem safety net to a separate class.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user StephanEwen opened a pull request: https://github.com/apache/flink/pull/3328 FLINK-5812 [core] Cleanups in the FileSystem class The FileSystem class is overloaded and has methods that are not well supported. This pull request does the following cleanups: Commit 1: Pull the safety net into a separate class Extend the docs of the safety net. Commit 2: Use the `WriteMode` to indicate overwriting behavior. Right now, the `FileSystem` class defines that enum and never uses it. It feels weird. Remove the method `FsDataOutputStream create(path, overwrite, blocksize, replication, ...)`, which is not really supported across file system implementations. For HDFS, the behavior should be set via the configuration anyways. All changes have to be made in a non-API-breaking fashion. You can merge this pull request into a Git repository by running: $ git pull https://github.com/StephanEwen/incubator-flink fs_cleanup Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3328.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3328 commit 6c86f901427d4bb077f27c84c90abeafec019449 Author: Stephan Ewen <sewen@apache.org> Date: 2017-02-15T16:10:53Z FLINK-5812 [core] Cleanups in FileSystem (round 1) This makes the FileSystem use the 'WriteMode' (otherwise it was an unused enumeration) Extends comments Deprecate the method that controls the replication factor and block size commit 83578e2b8e2e1ca03b14e8d47f7a122aa07fd517 Author: Stephan Ewen <sewen@apache.org> Date: 2017-02-15T16:58:37Z FLINK-5812 [core] Cleanups in FileSystem (round 2) Move the FileSystem safety net to a separate class.
          Hide
          StephanEwen Stephan Ewen added a comment -

          Fixed via 5902ea0e88c70f330c23b9ace94033ae34c84445

          Show
          StephanEwen Stephan Ewen added a comment - Fixed via 5902ea0e88c70f330c23b9ace94033ae34c84445
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user zentol commented on the issue:

          https://github.com/apache/flink/pull/3328

          @StephanEwen Can this PR be closed?

          Show
          githubbot ASF GitHub Bot added a comment - Github user zentol commented on the issue: https://github.com/apache/flink/pull/3328 @StephanEwen Can this PR be closed?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen commented on the issue:

          https://github.com/apache/flink/pull/3328

          Yes, it was accidentally merged together with another PR.
          Since the changes were mostly comments and annotations, it should not be a problem.

          Closing...

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen commented on the issue: https://github.com/apache/flink/pull/3328 Yes, it was accidentally merged together with another PR. Since the changes were mostly comments and annotations, it should not be a problem. Closing...
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user StephanEwen closed the pull request at:

          https://github.com/apache/flink/pull/3328

          Show
          githubbot ASF GitHub Bot added a comment - Github user StephanEwen closed the pull request at: https://github.com/apache/flink/pull/3328

            People

            • Assignee:
              StephanEwen Stephan Ewen
              Reporter:
              StephanEwen Stephan Ewen
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development