Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove default value for hostid parameter #26

Merged
merged 12 commits into from
Oct 24, 2023
9 changes: 5 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,25 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: ["3.7", "3.10"]
python-version: ["3.7", "3.8", "3.9", "3.10"]
runs-on: [ubuntu-latest]

steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- name: Set up Conda
uses: conda-incubator/setup-miniconda@v2.1.1
uses: conda-incubator/setup-miniconda@v2.2.0
with:
mamba-version: "*"
python-version: ${{ matrix.python-version }}
miniforge-variant: Mambaforge
channels: conda-forge,defaults
channel-priority: true
activate-environment: fsspec
- name: Install build environment
shell: bash -l {0}
run: |
mamba install -c conda-forge python=${{ matrix.python-version }} "xrootd>=5.4.3"
mamba install -c conda-forge "xrootd>=5.4.3"
- name: Install package
shell: bash -l {0}
run: python -m pip install .[test]
Expand Down
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ repos:
additional_dependencies: [black==22.3.0]

- repo: https://github.com/PyCQA/isort
rev: 5.10.1
rev: 5.12.0
hooks:
- id: isort
args: ["-a", "from __future__ import annotations"] # Python 3.7+
Expand Down Expand Up @@ -84,7 +84,7 @@ repos:
additional_dependencies: *flake8_dependencies

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.961
rev: v1.4.1
hooks:
- id: mypy
files: src
Expand Down
10 changes: 7 additions & 3 deletions src/fsspec_xrootd/xrootd.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from __future__ import annotations

Check warning on line 1 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Missing module docstring

import asyncio
import io
Expand All @@ -15,18 +15,18 @@
sync_wrapper,
)
from fsspec.spec import AbstractBufferedFile # type: ignore[import]
from XRootD import client # type: ignore[import]

Check failure on line 18 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Unable to import 'XRootD'
from XRootD.client.flags import ( # type: ignore[import]

Check failure on line 19 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Unable to import 'XRootD.client.flags'
DirListFlags,
MkDirFlags,
OpenFlags,
QueryCode,
StatInfoFlags,
)
from XRootD.client.responses import HostList, XRootDStatus # type: ignore[import]

Check failure on line 26 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Unable to import 'XRootD.client.responses'


class ErrorCodes(IntEnum):

Check warning on line 29 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Missing class docstring
INVALID_PATH = 400


Expand All @@ -34,7 +34,7 @@
future: asyncio.Future[tuple[XRootDStatus, Any]],
status: XRootDStatus,
content: Any,
servers: HostList,

Check warning on line 37 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Unused argument 'servers'
) -> None:
"""Sets result of _async_wrap() future.

Expand All @@ -53,7 +53,7 @@
return
try:
future.get_loop().call_soon_threadsafe(future.set_result, (status, content))
except Exception as exc:

Check warning on line 56 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Catching too general exception Exception
future.get_loop().call_soon_threadsafe(future.set_exception, exc)


Expand Down Expand Up @@ -141,7 +141,7 @@
return deets


class XRootDFileSystem(AsyncFileSystem): # type: ignore[misc]

Check warning on line 144 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Missing class docstring

Check warning on line 144 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Method '_cp_file' is abstract in class 'AsyncFileSystem' but is not overridden in child class 'XRootDFileSystem'

Check warning on line 144 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Method '_get_file' is abstract in class 'AsyncFileSystem' but is not overridden in child class 'XRootDFileSystem'

Check warning on line 144 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Method '_pipe_file' is abstract in class 'AsyncFileSystem' but is not overridden in child class 'XRootDFileSystem'

Check warning on line 144 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Method '_put_file' is abstract in class 'AsyncFileSystem' but is not overridden in child class 'XRootDFileSystem'

Check warning on line 144 in src/fsspec_xrootd/xrootd.py

View workflow job for this annotation

GitHub Actions / Format

Method 'cp_file' is abstract in class 'AbstractFileSystem' but is not overridden in child class 'XRootDFileSystem'

protocol = "root"
root_marker = "/"
Expand All @@ -152,7 +152,7 @@

def __init__(
self,
hostid: str = "",
hostid: str,
asynchronous: bool = False,
loop: Any = None,
**storage_options: Any,
Expand All @@ -174,7 +174,7 @@
self.timeout = storage_options.get("timeout", XRootDFileSystem.default_timeout)
self._myclient = client.FileSystem("root://" + hostid)
if not self._myclient.url.is_valid():
raise ValueError(f"Invalid hostid: '{hostid}'")
raise ValueError(f"Invalid hostid: {hostid!r}")
storage_options.setdefault("listing_expiry_time", 0)
self.storage_options = storage_options

Expand All @@ -196,7 +196,10 @@
@classmethod
def _strip_protocol(cls, path: str | list[str]) -> Any:
if type(path) == str:
return client.URL(path).path or cls.root_marker
if path.startswith(cls.protocol):
return client.URL(path).path.rstrip("/") or cls.root_marker
# assume already stripped
return path.rstrip("/") or cls.root_marker
elif type(path) == list:
return [cls._strip_protocol(item) for item in path]
else:
Expand Down Expand Up @@ -676,6 +679,7 @@
" deprecated. "
"Specify it within the 'cache_options' argument instead.",
FutureWarning,
stacklevel=1,
)
cache_options["trim"] = kwargs.pop("trim")

Expand Down
26 changes: 23 additions & 3 deletions tests/test_basicio.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
import fsspec
import pytest

from fsspec_xrootd.xrootd import _chunks_to_vectors, _vectors_to_chunks
from fsspec_xrootd.xrootd import (
XRootDFileSystem,
_chunks_to_vectors,
_vectors_to_chunks,
)

TESTDATA1 = "apple\nbanana\norange\ngrape"
TESTDATA2 = "red\ngreen\nyellow\nblue"
Expand Down Expand Up @@ -53,6 +57,17 @@ def test_invalid_server():
fsspec.core.url_to_fs("root://")


def test_invalid_parameters():
with pytest.raises(TypeError):
fsspec.filesystem(protocol="root")


def test_async_impl():
cls = fsspec.get_filesystem_class(protocol="root")
assert cls == XRootDFileSystem
assert cls.async_impl, "XRootDFileSystem should have async_impl=True"


def test_broken_server():
with pytest.raises(OSError):
# try to connect on the wrong port should fail
Expand All @@ -71,9 +86,14 @@ def test_path_parsing():
fs, _, (path,) = fsspec.get_fs_token_paths("root://server.com//blah")
assert path == "/blah"
fs, _, paths = fsspec.get_fs_token_paths(
["root://server.com//blah", "root://server.com//more"]
[
"root://server.com//blah",
"root://server.com//more",
"root://server.com/dir/",
"root://serv.er//dir/",
]
)
assert paths == ["/blah", "/more"]
assert paths == ["/blah", "/more", "dir", "/dir"]


def test_pickle(localserver, clear_server):
Expand Down
Loading