Repository: fsspec/filesystem_spec

Issue:
Generic Filesystem does not await call to async function
**Describe the problem**
In generic.py on line 173, the function _cp_file of GenericFileSystem makes the following call:
`callback.set_size(maybe_await(f1.size))`
maybe_await is a wrapper that decides whether or not it's parameter is a Coroutine, and therefore if the parameter should be called with await or not. The maybe_await function itself is an asynchronous function, and thus the call to it on line 173 should be awaited, which it is currently not.

**Propose a solution**
Simply add the await statement in front of the call to maybe_await:
`callback.set_size(await maybe_await(f1.size))`

Discussion:
martindurant: +1
Clearly this line was not covered by tests. Do you think you con contrive a test which does touch it?

Patch:
diff --git a/docs/environment.yml b/docs/environment.yml
index b5f5a9d..814bd85 100644
--- a/docs/environment.yml
+++ b/docs/environment.yml
@@ -5,3 +5,4 @@ dependencies:
   - python=3.9
   - docutils<0.17
   - numpydoc
+  - yarl
diff --git a/fsspec/generic.py b/fsspec/generic.py
index 360f59a..2796048 100644
--- a/fsspec/generic.py
+++ b/fsspec/generic.py
@@ -170,7 +170,7 @@ class GenericFileSystem(AsyncFileSystem):
                 if hasattr(fs, "open_async")
                 else fs.open(url, "rb", **kw)
             )
-            callback.set_size(maybe_await(f1.size))
+            callback.set_size(await maybe_await(f1.size))
             f2 = (
                 await fs2.open_async(url2, "wb")
                 if hasattr(fs2, "open_async")
diff --git a/fsspec/implementations/arrow.py b/fsspec/implementations/arrow.py
index 0edf8c5..803d44b 100644
--- a/fsspec/implementations/arrow.py
+++ b/fsspec/implementations/arrow.py
@@ -59,6 +59,7 @@ class ArrowFSWrapper(AbstractFileSystem):
         return path
 
     def ls(self, path, detail=False, **kwargs):
+        path = self._strip_protocol(path)
         from pyarrow.fs import FileSelector
 
         entries = [
@@ -184,6 +185,11 @@ class ArrowFSWrapper(AbstractFileSystem):
         path = self._strip_protocol(path)
         self.fs.delete_dir(path)
 
+    @wrap_exceptions
+    def modified(self, path):
+        path = self._strip_protocol(path)
+        return self.fs.get_file_info(path).mtime
+
 
 @mirror_from(
     "stream", ["read", "seek", "tell", "write", "readable", "writable", "close", "size"]
diff --git a/fsspec/implementations/cached.py b/fsspec/implementations/cached.py
index a0799a0..0619d1a 100644
--- a/fsspec/implementations/cached.py
+++ b/fsspec/implementations/cached.py
@@ -232,6 +232,44 @@ class CachingFileSystem(AbstractFileSystem):
         rmtree(self.storage[-1])
         self.load_cache()
 
+    def clear_expired_cache(self, expiry_time=None):
+        """Remove all expired files and metadata from the cache
+
+        In the case of multiple cache locations, this clears only the last one,
+        which is assumed to be the read/write one.
+
+        Parameters
+        ----------
+        expiry_time: int
+            The time in seconds after which a local copy is considered useless.
+            If not defined the default is equivalent to the attribute from the
+            file caching instantiation.
+        """
+
+        if not expiry_time:
+            expiry_time = self.expiry
+
+        self._check_cache()
+
+        for path, detail in self.cached_files[-1].copy().items():
+            if time.time() - detail["time"] > expiry_time:
+                if self.same_names:
+                    basename = os.path.basename(detail["original"])
+                    fn = os.path.join(self.storage[-1], basename)
+                else:
+                    fn = os.path.join(self.storage[-1], detail["fn"])
+                if os.path.exists(fn):
+                    os.remove(fn)
+                    self.cached_files[-1].pop(path)
+
+        if self.cached_files[-1]:
+            cache_path = os.path.join(self.storage[-1], "cache")
+            with open(cache_path, "wb") as fc:
+                pickle.dump(self.cached_files[-1], fc)
+        else:
+            rmtree(self.storage[-1])
+            self.load_cache()
+
     def pop_from_cache(self, path):
         """Remove cached version of given file
 
@@ -389,6 +427,7 @@ class CachingFileSystem(AbstractFileSystem):
             "_check_cache",
             "_mkcache",
             "clear_cache",
+            "clear_expired_cache",
             "pop_from_cache",
             "_mkcache",
             "local_file",
diff --git a/fsspec/implementations/memory.py b/fsspec/implementations/memory.py
index 09df148..cf0b2c5 100644
--- a/fsspec/implementations/memory.py
+++ b/fsspec/implementations/memory.py
@@ -40,7 +40,7 @@ class MemoryFileSystem(AbstractFileSystem):
                     "name": path,
                     "size": self.store[path].size,
                     "type": "file",
-                    "created": self.store[path].created,
+                    "created": self.store[path].created.timestamp(),
                 }
             ]
         paths = set()
@@ -55,7 +55,7 @@ class MemoryFileSystem(AbstractFileSystem):
                             "name": p2,
                             "size": self.store[p2].size,
                             "type": "file",
-                            "created": self.store[p2].created,
+                            "created": self.store[p2].created.timestamp(),
                         }
                     )
                 elif len(p2) > len(starter):
@@ -221,6 +221,20 @@ class MemoryFileSystem(AbstractFileSystem):
         except KeyError as e:
             raise FileNotFoundError(path) from e
 
+    def modified(self, path):
+        path = self._strip_protocol(path)
+        try:
+            return self.store[path].modified
+        except KeyError:
+            raise FileNotFoundError(path)
+
+    def created(self, path):
+        path = self._strip_protocol(path)
+        try:
+            return self.store[path].created
+        except KeyError:
+            raise FileNotFoundError(path)
+
     def rm(self, path, recursive=False, maxdepth=None):
         if isinstance(path, str):
             path = self._strip_protocol(path)
@@ -252,7 +266,8 @@ class MemoryFile(BytesIO):
         logger.debug("open file %s", path)
         self.fs = fs
         self.path = path
-        self.created = datetime.utcnow().timestamp()
+        self.created = datetime.utcnow()
+        self.modified = datetime.utcnow()
         if data:
             super().__init__(data)
             self.seek(0)
@@ -272,3 +287,4 @@ class MemoryFile(BytesIO):
 
     def commit(self):
         self.fs.store[self.path] = self
+        self.modified = datetime.utcnow()


Test patch:
diff --git a/fsspec/implementations/tests/conftest.py b/fsspec/implementations/tests/conftest.py
index 70d79ef..7dce3ee 100644
--- a/fsspec/implementations/tests/conftest.py
+++ b/fsspec/implementations/tests/conftest.py
@@ -2,21 +2,33 @@ import tempfile
 
 import pytest
 
+from fsspec.implementations.arrow import ArrowFSWrapper
 from fsspec.implementations.local import LocalFileSystem
-
+from fsspec.implementations.memory import MemoryFileSystem
 
 # A dummy filesystem that has a list of protocols
+
+
 class MultiProtocolFileSystem(LocalFileSystem):
     protocol = ["file", "other"]
 
 
-FILESYSTEMS = {"local": LocalFileSystem, "multi": MultiProtocolFileSystem}
+FILESYSTEMS = {
+    "local": LocalFileSystem,
+    "multi": MultiProtocolFileSystem,
+    "memory": MemoryFileSystem,
+}
 
 READ_ONLY_FILESYSTEMS = []
 
 
 @pytest.fixture(scope="function")
 def fs(request):
+    pyarrow_fs = pytest.importorskip("pyarrow.fs")
+    FileSystem = pyarrow_fs.FileSystem
+    if request.param == "arrow":
+        fs = ArrowFSWrapper(FileSystem.from_uri("file:///")[0])
+        return fs
     cls = FILESYSTEMS[request.param]
     return cls()
 
diff --git a/fsspec/implementations/tests/test_arrow.py b/fsspec/implementations/tests/test_arrow.py
index 1386df8..99d8a49 100644
--- a/fsspec/implementations/tests/test_arrow.py
+++ b/fsspec/implementations/tests/test_arrow.py
@@ -14,11 +14,11 @@ def fs():
     return ArrowFSWrapper(fs)
 
 
-@pytest.fixture(scope="function")
-def remote_dir(fs):
+@pytest.fixture(scope="function", params=[False, True])
+def remote_dir(fs, request):
     directory = secrets.token_hex(16)
     fs.makedirs(directory)
-    yield directory
+    yield ("hdfs://" if request.param else "") + directory
     fs.rm(directory, recursive=True)
 
 
@@ -30,18 +30,19 @@ def strip_keys(original_entry):
 
 def test_info(fs, remote_dir):
     fs.touch(remote_dir + "/a.txt")
+    remote_dir_strip_protocol = fs._strip_protocol(remote_dir)
     details = fs.info(remote_dir + "/a.txt")
     assert details["type"] == "file"
-    assert details["name"] == remote_dir + "/a.txt"
+    assert details["name"] == remote_dir_strip_protocol + "/a.txt"
     assert details["size"] == 0
 
     fs.mkdir(remote_dir + "/dir")
     details = fs.info(remote_dir + "/dir")
     assert details["type"] == "directory"
-    assert details["name"] == remote_dir + "/dir"
+    assert details["name"] == remote_dir_strip_protocol + "/dir"
 
     details = fs.info(remote_dir + "/dir/")
-    assert details["name"] == remote_dir + "/dir/"
+    assert details["name"] == remote_dir_strip_protocol + "/dir/"
 
 
 def test_move(fs, remote_dir):
@@ -114,12 +115,14 @@ def test_rm(fs, remote_dir):
 
 
 def test_ls(fs, remote_dir):
+    remote_dir_strip_protocol = fs._strip_protocol(remote_dir)
     fs.mkdir(remote_dir + "dir/")
     files = set()
     for no in range(8):
         file = remote_dir + f"dir/test_{no}"
+        # we also want to make sure `fs.touch` works with protocol
         fs.touch(file)
-        files.add(file)
+        files.add(remote_dir_strip_protocol + f"dir/test_{no}")
 
     assert set(fs.ls(remote_dir + "dir/")) == files
 
diff --git a/fsspec/implementations/tests/test_cached.py b/fsspec/implementations/tests/test_cached.py
index 4ddc051..51c451e 100644
--- a/fsspec/implementations/tests/test_cached.py
+++ b/fsspec/implementations/tests/test_cached.py
@@ -153,6 +153,109 @@ def test_clear():
     assert len(os.listdir(cache1)) < 2
 
 
+def test_clear_expired(tmp_path):
+    def __ager(cache_fn, fn):
+        """
+        Modify the cache file to virtually add time lag to selected files.
+
+        Parameters
+        ---------
+        cache_fn: str
+            cache path
+        fn: str
+            file name to be modified
+        """
+        import pathlib
+        import time
+
+        if os.path.exists(cache_fn):
+            with open(cache_fn, "rb") as f:
+                cached_files = pickle.load(f)
+                fn_posix = pathlib.Path(fn).as_posix()
+                cached_files[fn_posix]["time"] = cached_files[fn_posix]["time"] - 691200
+            assert os.access(cache_fn, os.W_OK), "Cache is not writable"
+            with open(cache_fn, "wb") as f:
+                pickle.dump(cached_files, f)
+            time.sleep(1)
+
+    origin = tmp_path.joinpath("origin")
+    cache1 = tmp_path.joinpath("cache1")
+    cache2 = tmp_path.joinpath("cache2")
+    cache3 = tmp_path.joinpath("cache3")
+
+    origin.mkdir()
+    cache1.mkdir()
+    cache2.mkdir()
+    cache3.mkdir()
+
+    data = b"test data"
+    f1 = origin.joinpath("afile")
+    f2 = origin.joinpath("bfile")
+    f3 = origin.joinpath("cfile")
+    f4 = origin.joinpath("dfile")
+
+    with open(f1, "wb") as f:
+        f.write(data)
+    with open(f2, "wb") as f:
+        f.write(data)
+    with open(f3, "wb") as f:
+        f.write(data)
+    with open(f4, "wb") as f:
+        f.write(data)
+
+    # populates first cache
+    fs = fsspec.filesystem(
+        "filecache", target_protocol="file", cache_storage=str(cache1), cache_check=1
+    )
+    assert fs.cat(str(f1)) == data
+
+    # populates "last" cache if file not found in first one
+    fs = fsspec.filesystem(
+        "filecache",
+        target_protocol="file",
+        cache_storage=[str(cache1), str(cache2)],
+        cache_check=1,
+    )
+    assert fs.cat(str(f2)) == data
+    assert fs.cat(str(f3)) == data
+    assert len(os.listdir(cache2)) == 3
+
+    # force the expiration
+    cache_fn = os.path.join(fs.storage[-1], "cache")
+    __ager(cache_fn, f2)
+
+    # remove from cache2 the expired files
+    fs.clear_expired_cache()
+    assert len(os.listdir(cache2)) == 2
+
+    # check complete cleanup
+    __ager(cache_fn, f3)
+
+    fs.clear_expired_cache()
+    assert not fs._check_file(f2)
+    assert not fs._check_file(f3)
+    assert len(os.listdir(cache2)) < 2
+
+    # check cache1 to be untouched after cleaning
+    assert len(os.listdir(cache1)) == 2
+
+    # check cleaning with 'same_name' option enabled
+    fs = fsspec.filesystem(
+        "filecache",
+        target_protocol="file",
+        cache_storage=[str(cache1), str(cache2), str(cache3)],
+        same_names=True,
+        cache_check=1,
+    )
+    assert fs.cat(str(f4)) == data
+
+    cache_fn = os.path.join(fs.storage[-1], "cache")
+    __ager(cache_fn, f4)
+
+    fs.clear_expired_cache()
+    assert not fs._check_file(str(f4))
+
+
 def test_pop():
     import tempfile
 
diff --git a/fsspec/implementations/tests/test_common.py b/fsspec/implementations/tests/test_common.py
index 4417213..fe88dc3 100644
--- a/fsspec/implementations/tests/test_common.py
+++ b/fsspec/implementations/tests/test_common.py
@@ -18,14 +18,15 @@ def test_created(fs: AbstractFileSystem, temp_file):
             fs.rm(temp_file)
 
 
-@pytest.mark.parametrize("fs", ["local"], indirect=["fs"])
+@pytest.mark.parametrize("fs", ["local", "memory", "arrow"], indirect=["fs"])
 def test_modified(fs: AbstractFileSystem, temp_file):
     try:
         fs.touch(temp_file)
-        created = fs.created(path=temp_file)
+        # created = fs.created(path=temp_file)
+        created = datetime.datetime.utcnow()  # pyarrow only have modified
         time.sleep(0.05)
         fs.touch(temp_file)
-        modified = fs.modified(path=temp_file)
+        modified = fs.modified(path=temp_file).replace(tzinfo=None)
         assert isinstance(modified, datetime.datetime)
         assert modified > created
     finally: