[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-04-16 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-93728544 This issue is discussed in FLINK-1848. --- 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

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-04-16 Thread balidani
Github user balidani commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-93723421 Hi! This PR was closed, but the commit right after mine removes the part that fixed the windows issue.

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-04-07 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-90507970 Btw, the path `file:/c:` that motivated this PR is not a valid absolute path according to Microsoft's documentation. --- If your project is set up for it, you can reply

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-04-04 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-89548376 I checked what are [valid absolute paths on windows](https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx#paths). Summary: A

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-27 Thread balidani
Github user balidani commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-87062926 So most of the jobs passed, except for PROFILE=-Dhadoop.version=2.6.0 -Dscala-2.11, where it says No output has been received in the last 10 minutes, this

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-25 Thread balidani
Github user balidani commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-86219748 Thanks, I did that, hopefully the tests will pass now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-25 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-86199581 I had a look at the Travis logs and it seems that the build failures are not related to your change. We had a few issues with build stability on the master branch in

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-19 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-83453500 Before merging this, we should find out what are valid windows paths. The tests should be defined based on that, not based on what the `Path` class accepts ;-) ---

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-19 Thread balidani
Github user balidani commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-83666214 I doubt that paths like `/C:/...` are valid on Windows. I guess Path should be changed then? How can we tell whether a path comes from Windows or not? The Travis

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-17 Thread balidani
Github user balidani commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-82586976 Yeah, that makes sense, I added two more assertions. Sorry, I forgot to check checkstyle before committing. Should I squash these commits in the end? --- If

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-17 Thread balidani
Github user balidani commented on a diff in the pull request: https://github.com/apache/flink/pull/491#discussion_r26613854 --- Diff: flink-core/src/test/java/org/apache/flink/core/fs/PathTest.java --- @@ -63,6 +63,15 @@ public void testPathFromString() {

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-17 Thread balidani
Github user balidani commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-82625011 Hi! I squashed them, I hope it's done correctly. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-17 Thread balidani
Github user balidani commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-82567110 Hi! I added some test cases to `PathTest`. Is it okay like this? --- If your project is set up for it, you can reply to this email and have your reply appear on

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-17 Thread balidani
Github user balidani commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-82577438 I just realized that this fails in Travis, because in `hasWindowsDrive` (Path.java, line 282) there is `if (!OperatingSystem.isWindows()) {`. What can we do in this

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-17 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-82578379 I think that we should identify windows drives independently of whether we are on windows. The client may run on Linux, the cluster windows (or vice versa), that is

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-17 Thread balidani
Github user balidani commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-82580762 I removed that part, I hope you meant it like this :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-17 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-82584339 Yeah, that is a good start. Does it make sense to also check for the pattern driverletter-colon (c:) ? --- If your project is set up for it, you can reply to this

[GitHub] flink pull request: Fix issue where Windows paths were not recogni...

2015-03-17 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-82588549 Yeah, squashing would be good... --- 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