diff --git a/obstore/python/obstore/fsspec.py b/obstore/python/obstore/fsspec.py index 5587b1ec..15fe32e0 100644 --- a/obstore/python/obstore/fsspec.py +++ b/obstore/python/obstore/fsspec.py @@ -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 @@ -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 @@ -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. @@ -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("/")) return ( "", parsed_path, @@ -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) @@ -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 @@ -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 diff --git a/tests/test_fsspec.py b/tests/test_fsspec.py index 8bf490ef..ad5889de 100644 --- a/tests/test_fsspec.py +++ b/tests/test_fsspec.py @@ -361,26 +361,24 @@ def test_split_path(fs: FsspecStore): with pytest.raises(ValueError, match=r"Expected protocol to be s3. Got gs"): fs._split_path("gs://data-bucket/") - # in url format, without bucket + # Paths without a bucket keep their existing POSIX behavior. Windows + # drive splitting and file operations are covered by the focused test below. 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/") - - # 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": + assert file_fs._split_path("file:///mybucket/path/to/file") == ( + "", + "/mybucket/path/to/file", + ) + assert file_fs._split_path("file:///data-bucket/") == ("", "/data-bucket/") + assert file_fs._split_path("/mybucket/path/to/file") == ( + "", + "/mybucket/path/to/file", + ) + assert file_fs._split_path("/data-bucket/") == ("", "/data-bucket/") + 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") # memory protocol keeps relative paths untouched (paths are virtual) memory_fs = FsspecStore("memory") # type: ignore (no __init__ overload for memory) @@ -390,6 +388,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], ):