fsspec.generic.rsync(<s3_path>, <s3_path>) raises FileNotFoundError

[mukhery]

When trying to use fsspec.generic.rsync to copy a remote s3 path to another s3 path a FileNotFoundError is raised.

  File "/home/ryan/micromamba/envs/testenv/lib/python3.11/site-packages/fsspec/generic.py", line 355, in _copy
    return await fs._copy(path1, path2, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ryan/micromamba/envs/testenv/lib/python3.11/site-packages/fsspec/asyn.py", line 405, in _copy
    raise ex
  File "/home/ryan/micromamba/envs/testenv/lib/python3.11/site-packages/fsspec/asyn.py", line 245, in _run_coro
    return await asyncio.wait_for(coro, timeout=timeout), i
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ryan/micromamba/envs/testenv/lib/python3.11/asyncio/tasks.py", line 452, in wait_for
    return await fut
           ^^^^^^^^^
  File "/home/ryan/micromamba/envs/testenv/lib/python3.11/site-packages/s3fs/core.py", line 1904, in _cp_file
    info = await self._info(path1, bucket, key, version_id=vers)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ryan/micromamba/envs/testenv/lib/python3.11/site-packages/s3fs/core.py", line 1355, in _info
    out = self._ls_from_cache(fullpath)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ryan/micromamba/envs/testenv/lib/python3.11/site-packages/fsspec/spec.py", line 374, in _ls_from_cache
    raise FileNotFoundError(path)
FileNotFoundError: testbucket/rsynctest/test_file.txt

@martindurant Should this line

s3fs/s3fs/core.py

Line 1343 in a28863f

fullpath = "/".join((bucket, key))

be fullpath = "s3://".join((bucket, key))? Making that change seems to fix things.

[martindurant]

No, the interior paths dealt with by s3fs (and almost all other backends) do not include the protocol, but "bucket/path/file"; most methods also strip the protocol prefix on the first line.

Noting that

>>> "s3://".join(("bucket", "key"))
'buckets3://key'

I am puzzled how this might have worked for you.

[mukhery]

I am puzzled how this might have worked for you.

lol! yeah that was a silly "fix" on my part...

It looks like the reason it worked is because it causes

s3fs/s3fs/core.py

Line 1355 in a28863f

out = self._ls_from_cache(fullpath)

to always throw KeyErrors which are caught and ignored by
https://github.com/fsspec/filesystem_spec/blob/59a2d648b43c12a5db7b7eac834d7af33df8a1fe/fsspec/spec.py#L376 which then allows execution to reach

s3fs/s3fs/core.py

Lines 1372 to 1391 in a28863f

	if key:
	try:
	out = await self._call_s3(
	"head_object",
	self.kwargs,
	Bucket=bucket,
	Key=key,
	**version_id_kw(version_id),
	**self.req_kw,
	)
	return {
	"ETag": out.get("ETag", ""),
	"LastModified": out.get("LastModified", ""),
	"size": out["ContentLength"],
	"name": "/".join([bucket, key]),
	"type": "file",
	"StorageClass": out.get("StorageClass", "STANDARD"),
	"VersionId": out.get("VersionId"),
	"ContentType": out.get("ContentType"),
	}

In the current state (i.e., without breaking the following line)

s3fs/s3fs/core.py

Line 1355 in a28863f

out = self._ls_from_cache(fullpath)

execution reaches
https://github.com/fsspec/filesystem_spec/blob/59a2d648b43c12a5db7b7eac834d7af33df8a1fe/fsspec/spec.py#L372-L374
which raises the FileNotFoundError

[martindurant]

Note that the two paths required by rsync:

must be a directory, but do not include any terminating "/" character

Can you please introspect the state of fs.dircache at the time of your call? _ls_from_cache raising implies that the parent directory was already listed, but the requested file does not exist in it.

[mukhery]

@martindurant Looks like path is testbucket/rsynctest/testfile1.txt

dircache[parent] is [{'Key': 'testbucket/rsynctest/testfile1.txt', 'LastModified': datetime.datetime(2024, 6, 6, 15, 18, 4, tzinfo=tzutc()), 'ETag': '"54b0c58c7ce9f2a8b551351102ee0938"', 'Size': 14, 'StorageClass': 'STANDARD', 'type': 'file', 'size': 14, 'name': 's3://testbucket/rsynctest/testfile1.txt'}, {'Key': 'testbucket/rsynctest/testfile2.txt', 'LastModified': datetime.datetime(2024, 6, 6, 15, 18, 10, tzinfo=tzutc()), 'ETag': '"d158a5494cf76bf2cbbe40a7aa674543"', 'Size': 20, 'StorageClass': 'STANDARD', 'type': 'file', 'size': 20, 'name': 's3://testbucket/rsynctest/testfile2.txt'}, {'Key': 'testbucket/rsynctest/testfile3.txt', 'LastModified': datetime.datetime(2024, 6, 6, 15, 18, 15, tzinfo=tzutc()), 'ETag': '"d285a2d347411ad4726fc350077138be"', 'Size': 24, 'StorageClass': 'STANDARD', 'type': 'file', 'size': 24, 'name': 's3://testbucket/rsynctest/testfile3.txt'}]

So it looks like

files = [
    f
    for f in self.dircache[parent]
    if f["name"] == path
    or (f["name"] == path.rstrip("/") and f["type"] == "directory")
]

is checking f["name"] which includes the s3:// prefix against path which does not include the prefix, but either needs to check against f["Key"] or add the prefix to path if continuing to use f["name"]

PS. I can confirm that my paths passed to rsync do not have terminating / characters

[mukhery]

@martindurant thoughts on this? Should I PR a check on "Key" or is something else wrong here?

[martindurant]

I think this is it:

--- a/fsspec/generic.py
+++ b/fsspec/generic.py
@@ -197,6 +197,7 @@ class GenericFileSystem(AsyncFileSystem):
             )
         result = {}
         for k, v in out.items():
+            v = v.copy()  # don't corrupt target FS dircache
             name = fs.unstrip_protocol(k)
             v["name"] = name
             result[name] = v

[martindurant]

(actually, I see there are a couple of other instances of this in the class; the one I picked is probably the one you were bitten by)