You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2017/01/05 13:59:37 UTC

[GitHub] flink pull request #3064: [FLINK-4890] [core] Make GlobFilePathFilter work o...

GitHub user StephanEwen opened a pull request:

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

    [FLINK-4890] [core] Make GlobFilePathFilter work on Windows

    When extracting the file path from the Flink `Path` object, this de-slashifies the path, if it is a slashed windows path.
    
    This is analogous to the logic in `Path.toString()`.
    
    The commit contains a minor cleanup in `FilePathFilter` to improve comments and fix serializability warnings.

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

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

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

    https://github.com/apache/flink/pull/3064.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 #3064
    
----
commit 2d099cc2f3e18b081ba59ca863a562be636cab7a
Author: Stephan Ewen <se...@apache.org>
Date:   2017-01-05T13:44:00Z

    [FLINK-4890] [core] Make GlobFilePathFilter work on Windows

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3064: [FLINK-4890] [core] Make GlobFilePathFilter work on Windo...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/3064
  
    You are right, the logic is the other way around than a predicate like in `FilterFunction`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3064: [FLINK-4890] [core] Make GlobFilePathFilter work on Windo...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/3064
  
    Okay, will merge this then.
    
    BTW: I think the include/exclude logic is faulty. If a path matches no include pattern, it is still accepted.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3064: [FLINK-4890] [core] Make GlobFilePathFilter work on Windo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/3064
  
    I can confirm that the test no longer fails on Windows.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #3064: [FLINK-4890] [core] Make GlobFilePathFilter work o...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3064#discussion_r94791293
  
    --- Diff: flink-core/src/main/java/org/apache/flink/core/fs/Path.java ---
    @@ -274,24 +274,6 @@ private String normalizePath(String path) {
     	}
     
     	/**
    -	 * Checks if the provided path string contains a windows drive letter.
    -	 * 
    -	 * @param path
    -	 *        the path to check
    -	 * @param slashed
    -	 *        <code>true</code> to indicate the first character of the string is a slash, <code>false</code> otherwise
    -	 * @return <code>true</code> if the path string contains a windows drive letter, <code>false</code> otherwise
    -	 */
    -	private boolean hasWindowsDrive(String path, boolean slashed) {
    --- End diff --
    
    It does not really break it (previously private methods should not be considered), but the plugin complains anyways.
    Will slightly rewrite this to make the plugin happy.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3064: [FLINK-4890] [core] Make GlobFilePathFilter work on Windo...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/3064
  
    If no include filter matches then filterPath should return true, in which case it is excluded.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #3064: [FLINK-4890] [core] Make GlobFilePathFilter work on Windo...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/3064
  
    Manually merging...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #3064: [FLINK-4890] [core] Make GlobFilePathFilter work o...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3064#discussion_r94788199
  
    --- Diff: flink-core/src/main/java/org/apache/flink/core/fs/Path.java ---
    @@ -274,24 +274,6 @@ private String normalizePath(String path) {
     	}
     
     	/**
    -	 * Checks if the provided path string contains a windows drive letter.
    -	 * 
    -	 * @param path
    -	 *        the path to check
    -	 * @param slashed
    -	 *        <code>true</code> to indicate the first character of the string is a slash, <code>false</code> otherwise
    -	 * @return <code>true</code> if the path string contains a windows drive letter, <code>false</code> otherwise
    -	 */
    -	private boolean hasWindowsDrive(String path, boolean slashed) {
    --- End diff --
    
    apparently this breaks binary compatibility.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #3064: [FLINK-4890] [core] Make GlobFilePathFilter work o...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---