[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-93728981 Thanks! --- 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

[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 does

[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. https://github.com/apache/flink/commit/23fe00629e4b36e

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

2015-04-07 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/491 --- 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 enabl

[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 pat

[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 th

[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-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 ;-) --- I

[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 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() { assertEqua

[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 n

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

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

[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 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 e

[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 y

[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 to

[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 situa

[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 StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/491#issuecomment-82518745 Looks like a good fix. Could you add a test for this as well, to be sure that windows paths are recognized? Also a test that verifies that "isAbsolute()" still behaves

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

2015-03-17 Thread balidani
GitHub user balidani opened a pull request: https://github.com/apache/flink/pull/491 Fix issue where Windows paths were not recognized as absolute Hello! An issue caused all tests to fail on Windows, because the path `file:/C:` was not recognized as absolute. This pull-