Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions obstore/python/obstore/fsspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from __future__ import annotations

import asyncio
import os
import warnings
from collections import defaultdict
from functools import cached_property, lru_cache
Expand All @@ -46,7 +47,8 @@
from fsspec.implementations.local import make_path_posix

from obstore import open_reader, open_writer
from obstore.store import from_url
from obstore.exceptions import GenericError
from obstore.store import LocalStore, from_url

if TYPE_CHECKING:
from collections.abc import Coroutine, Iterable
Expand Down Expand Up @@ -108,6 +110,16 @@
"""A type hint for all supported protocols."""


def _is_windows_drive_path(path: str) -> bool:
"""Whether the input is a Windows drive path such as `D:/dir/file`.

`urlparse` parses the drive letter of such a path as the URL scheme, so
drive paths must be detected before any URL handling.
"""
parsed = urlparse(path)
return os.name == "nt" and len(parsed.scheme) == 1 and parsed.scheme.isalpha()


class FsspecStore(fsspec.asyn.AsyncFileSystem):
"""An fsspec implementation based on a obstore Store.

Expand Down Expand Up @@ -295,11 +307,29 @@ def _split_path(self, path: str) -> tuple[str, str]:

# If the protocol doesn't require buckets, return empty bucket and full path
if self.protocol in protocol_without_bucket:
if self.protocol == "file" and _is_windows_drive_path(path):
# A Windows drive path such as `D:\dir\file` or `D:/dir/file` is
# not a URL: `urlparse` would parse the drive letter as the URL
# scheme. Resolve the whole path and return the addressed drive
# as the bucket so that `_construct_store` can root a local
# store on that drive.
drive, _, rest = str(make_path_posix(path)).partition(":")
return (f"{drive}:", rest.lstrip("/"))
parsed_path = (
f"{parsed.netloc}/{parsed.path.lstrip('/')}" if parsed.scheme else path
)
if self.protocol == "file":
parsed_path = str(make_path_posix(parsed_path))
if os.name == "nt":
# On Windows, `make_path_posix` resolves any path to a
# drive-rooted path such as `D:/dir/file`. As the store for
# an empty bucket is rooted at the current drive, joining a
# drive-rooted path onto it would duplicate the drive;
# instead return the resolved drive as the bucket, as for
# drive paths above.
drive, _, rest = parsed_path.partition(":")
if len(drive) == 1 and drive.isalpha():
return (f"{drive}:", rest.lstrip("/"))
Comment on lines +310 to +332

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is extremely verbose and I don't think you should need successive checks for windows.

return (
"",
parsed_path,
Expand Down Expand Up @@ -330,6 +360,11 @@ def _split_path(self, path: str) -> tuple[str, str]:

def _construct_store(self, bucket: str) -> ObjectStore:
protocol = self.protocol if isinstance(self.protocol, str) else self.protocol[0]
if protocol == "file" and bucket:
# A `file://` URL cannot express a Windows drive root: the drive
# would be parsed as the URL authority, not the path. Root the
# local store on the drive directly.
return LocalStore(f"{bucket}/", **self._config_kwargs)
return from_url(
url=f"{protocol}://{bucket}",
config=self._config, # type: ignore (type narrowing)
Expand Down Expand Up @@ -485,7 +520,10 @@ async def _put_file(

async def _get_file(self, rpath: str, lpath: str, **_kwargs: Any) -> None:
res = urlparse(lpath)
if res.scheme or await self._local_store._isdir(lpath): # noqa: SLF001
if (
(res.scheme and not _is_windows_drive_path(lpath))
or await self._local_store._isdir(lpath) # noqa: SLF001
):
# lpath need to be local file and cannot contain scheme
return

Expand Down Expand Up @@ -520,6 +558,13 @@ async def _info(self, path: str, **_kwargs: Any) -> dict[str, Any]:
}
except FileNotFoundError:
pass
except GenericError:
# On Windows, opening a directory to stat it fails with access
# denied before the upstream is-directory check can report it as
# not found, as `head` does on POSIX. Fall through to the
# directory fallback below only for directories.
if self.protocol != "file" or not Path(path).is_dir(): # noqa: ASYNC240
raise

# Ref: https://github.com/fsspec/s3fs/blob/01b9c4b838b81375093ae1d78562edf6bdc616ea/s3fs/core.py#L1471-L1492
# We check to see if the path is a directory by attempting to list its
Expand Down
107 changes: 93 additions & 14 deletions tests/test_fsspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,24 +363,48 @@ def test_split_path(fs: FsspecStore):

# in url format, without bucket
file_fs = FsspecStore("file")
assert file_fs._split_path("file:///mybucket/path/to/file") == (
"",
"/mybucket/path/to/file",
)
assert file_fs._split_path("file:///data-bucket/") == ("", "/data-bucket/")
drive, cwd_rel = os.path.splitdrive(Path.cwd().as_posix())
cwd_rel = cwd_rel.lstrip("/")
if os.name == "nt":
# On Windows, every path resolves onto a drive and the addressed drive
# is returned as the bucket so that `_construct_store` can root the
# local store on that drive.
for input_path in ("file:///mybucket/path/to/file", "/mybucket/path/to/file"):
assert file_fs._split_path(input_path) == (
drive,
f"{cwd_rel}/mybucket/path/to/file",
)
# An explicitly addressed drive is preserved.
assert file_fs._split_path(f"{drive}/data-bucket/") == (
drive,
"data-bucket/",
)
else:
assert file_fs._split_path("file:///mybucket/path/to/file") == (
"",
"/mybucket/path/to/file",
)
assert file_fs._split_path("file:///data-bucket/") == ("", "/data-bucket/")

# path format, without bucket
assert file_fs._split_path("/mybucket/path/to/file") == (
"",
"/mybucket/path/to/file",
)
assert file_fs._split_path("/data-bucket/") == ("", "/data-bucket/")
# path format, without bucket
assert file_fs._split_path("/mybucket/path/to/file") == (
"",
"/mybucket/path/to/file",
)
assert file_fs._split_path("/data-bucket/") == ("", "/data-bucket/")

# relative path, without bucket -> resolved to absolute for the "file" protocol
rel_bucket, rel_path = file_fs._split_path("relative/path/to/file")
assert rel_bucket == ""
assert Path(rel_path).is_absolute()
assert rel_path.endswith("relative/path/to/file")
if os.name == "nt":
# The resolved drive is returned as the bucket; joining it back onto
# the path yields an absolute path.
assert rel_bucket == drive
assert rel_path == f"{cwd_rel}/relative/path/to/file"
assert Path(f"{drive}/{rel_path}").is_absolute()
else:
assert rel_bucket == ""
assert Path(rel_path).is_absolute()
assert rel_path.endswith("relative/path/to/file")

# memory protocol keeps relative paths untouched (paths are virtual)
memory_fs = FsspecStore("memory") # type: ignore (no __init__ overload for memory)
Expand All @@ -390,6 +414,61 @@ def test_split_path(fs: FsspecStore):
)


@pytest.mark.skipif(os.name != "nt", reason="Windows drive path handling")
def test_file_protocol_windows_drive_paths():
"""The fsspec ``file`` protocol supports Windows drive paths.

On Windows, every path addresses a drive; the drive is returned as the
bucket so that `_construct_store` can root the local store there, and
directories resolve through the listing fallback, matching the POSIX
behavior of `head` on a directory.

Ref: https://github.com/developmentseed/obstore/issues/763
Ref: https://github.com/developmentseed/obstore/issues/555
"""
register("file")
fs = fsspec.filesystem("file")

with TemporaryDirectory() as tmp:
posix_dir = Path(tmp).as_posix()
file_path = f"{posix_dir}/file.txt"
Path(tmp, "file.txt").write_text("data")
subdir = Path(tmp, "sub")
subdir.mkdir()
Path(tmp, "sub", "inner.txt").write_text("inner")

assert fs.exists(file_path)
assert not fs.exists(f"{posix_dir}/missing.txt")
assert fs.info(file_path)["size"] == 4
assert fs.cat_file(file_path) == b"data"
with fs.open(file_path) as f:
assert f.read() == b"data"

assert fs.exists(f"{posix_dir}/sub")
assert fs.isdir(f"{posix_dir}/sub")
assert not fs.isdir(file_path)

names = fs.ls(posix_dir, detail=False)
assert file_path in names
assert f"{posix_dir}/sub" in names
assert fs.ls(f"{posix_dir}/sub", detail=False) == [
f"{posix_dir}/sub/inner.txt",
]

fs.pipe_file(f"{posix_dir}/piped.txt", b"piped")
assert fs.cat_file(f"{posix_dir}/piped.txt") == b"piped"
with fs.open(f"{posix_dir}/written.txt", mode="wb") as f:
f.write(b"written")
assert fs.cat_file(f"{posix_dir}/written.txt") == b"written"

local_out = Path(tmp, "out.txt")
fs.get(file_path, str(local_out))
assert local_out.read_text() == "data"

fs.rm_file(f"{posix_dir}/piped.txt")
assert not fs.exists(f"{posix_dir}/piped.txt")


def test_list(
minio_bucket: tuple[S3Config, ClientConfig],
):
Expand Down
Loading