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

Fsspec: Convert async methods that open sync file handles to use LocalStore #258

Open
kylebarron opened this issue Feb 11, 2025 · 0 comments
Labels
good first issue Good for newcomers

Comments

@kylebarron
Copy link
Member

kylebarron commented Feb 11, 2025

Async functions should not open files with blocking methods like open.

# TODO: convert to use async file system methods using LocalStore
# Async functions should not open files with blocking methods like `open`
with open(lpath, "rb") as f: # noqa: ASYNC230
await obs.put_async(self.store, rpath, f)

# TODO: convert to use async file system methods using LocalStore
# Async functions should not open files with blocking methods like `open`
with open(lpath, "wb") as f: # noqa: ASYNC230
resp = await obs.get_async(self.store, rpath)
async for buffer in resp.stream():
f.write(buffer)

Instead we should use something like:

    async def _put_file(
        self,
        lpath: str,
        rpath: str,
        mode: str = "overwrite",  # noqa: ARG002
        **_kwargs: Any,
    ) -> None:
        local_store = LocalStore("/")
        local_file = await obs.get_async(local_store, lpath)
        await obs.put_async(self.store, rpath, local_file)
  • We may want to cache the construction of LocalStore("/")? It looks like constructing that only takes 1.22 μs on my machine, so maybe not important.

  • It looks like the leading "/" on a path is not an issue. I.e. this works even though there's a leading "/" in the path:

    lpath = "/Users/kyle/github/developmentseed/obstore/README.md"
    local_store = LocalStore("/")
    local_file = obs.get(local_store, lpath)
    print(local_file.bytes().to_bytes().decode())
  • This may deadlock in the case that the async implementation is used synchronously, but that's something we can come back to in the future.

@kylebarron kylebarron added the good first issue Good for newcomers label Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant