fix get_dataset_path() in fs_utils.py

[dongpohezui]

version:
Windows 10
Python 3.7.0
petastorm 0.9.8
pyarrow 3.0.0

from petastorm import make_reader

reader = make_reader('file:///D:/test/test.parquet') 
print(reader)

The following error occurred when I ran the above code under Windows.

The image above shows that there is an extra '/' at the beginning of the file path.
So I deleted the extra '/' .

[CLAassistant]

All committers have signed the CLA.

[codecov]

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 85.19%. Comparing base (10e0fc8) to head (271da38).
Report is 71 commits behind head on master.

Files with missing lines	Patch %	Lines
petastorm/fs_utils.py	33.33%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #663      +/-   ##
==========================================
- Coverage   85.22%   85.19%   -0.04%     
==========================================
  Files          85       85              
  Lines        4981     4984       +3     
  Branches      791      792       +1     
==========================================
+ Hits         4245     4246       +1     
- Misses        596      597       +1     
- Partials      140      141       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[selitvin]

Did you manage to track down what is the origin of this extra '/'? Which part of code added it? Asking this to make sure the same issue wouldn't pop up in some other scenarios with different code paths.

Also, can you please add a unittest covering this case?

[dongpohezui]

The URL comes from user input.
Under Linux, it works just fine.
Under Windows, it makes mistakes.

In get_filesystem_and_path_or_paths(), URLparse () is used to handle URLs.

Under Windows, the file cannot be accessed via the file path "/D:/test/test.parquet ". Instead, it should be "D:/test/test.parquet".

In my opinion, you can only test this code under Windows.

[selitvin]

There is a chance that this fix would break for file urls on network location (as per this article). Consider the example of the following WIndows url: file://hostname/path/to/the%20file.txt

[selitvin]

It would be great if you could add a unit test verifying correctness of this function in petastorm/tests/test_fs_utils.py. You would need to mock the value returned by platform.system() and then I think the test logic should be straight forward.

If for some reason you can not do this, we'd be able to land this and I can help with the test later.

[selitvin]

Marking the PR with "Request Changes" due to incorrect behavior in the case of windows urls that contain network and not local paths.