Skip to content

Commit

Permalink
Merge pull request #1376 from dandi/open-readable
Browse files Browse the repository at this point in the history
Properly open filehandles for `RemoteReadableAsset`s
  • Loading branch information
yarikoptic authored Dec 13, 2023
2 parents a51e0a7 + cef7192 commit 14d319c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 2 deletions.
5 changes: 4 additions & 1 deletion dandi/misctypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,10 @@ def open(self) -> IO[bytes]:
# Optional dependency:
import fsspec

return cast(IO[bytes], fsspec.open(self.url, mode="rb"))
# We need to call open() on the return value of fsspec.open() because
# otherwise the filehandle will only be opened when used to enter a
# context manager.
return cast(IO[bytes], fsspec.open(self.url, mode="rb").open())

def get_size(self) -> int:
return self.size
Expand Down
26 changes: 25 additions & 1 deletion dandi/tests/test_dandiapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@
dandiset_identifier_regex,
dandiset_metadata_file,
)
from ..dandiapi import DandiAPIClient, RemoteAsset, RemoteZarrAsset, Version
from ..dandiapi import (
DandiAPIClient,
RemoteAsset,
RemoteBlobAsset,
RemoteZarrAsset,
Version,
)
from ..download import download
from ..exceptions import NotFoundError, SchemaVersionError
from ..files import GenericAsset, dandi_file
Expand Down Expand Up @@ -745,3 +751,21 @@ def test_rename_type_mismatch(text_dandiset: SampleDandiset, dest: str) -> None:
assert asset1a.get_raw_metadata()["path"] == "file.txt"
with pytest.raises(NotFoundError):
text_dandiset.dandiset.get_asset_by_path(dest)


def test_asset_as_readable_open(new_dandiset: SampleDandiset, tmp_path: Path) -> None:
p = tmp_path / "foo.txt"
# Write bytes so that the LF doesn't get converted on Windows:
p.write_bytes(b"This is test text.\n")
d = new_dandiset.dandiset
d.upload_raw_asset(p, {"path": "foo.txt"})
(asset,) = d.get_assets()
assert isinstance(asset, RemoteBlobAsset)
# The purpose of this test is to check that RemoteReadableAsset.open()
# returns a filehandle that can immediately be used, so don't use a `with`
# block here, as that opens fsspec file objects.
fp = asset.as_readable().open()
try:
assert fp.read() == b"This is test text.\n"
finally:
fp.close()

0 comments on commit 14d319c

Please sign in to comment.