Fix on_error="omit" for cat via caching (#1202)

Author: martindurant

fsspec/implementations/cached.py

@@ -614,20 +614,28 @@ def cat(
         getpaths = []
         storepaths = []
         fns = []
-        for p in paths:
-            detail = self._check_file(p)
-            if not detail:
-                fn = self._make_local_details(p)
-                getpaths.append(p)
-                storepaths.append(fn)
-            else:
-                detail, fn = detail if isinstance(detail, tuple) else (None, detail)
-            fns.append(fn)
+        out = {}
+        for p in paths.copy():
+            try:
+                detail = self._check_file(p)
+                if not detail:
+                    fn = self._make_local_details(p)
+                    getpaths.append(p)
+                    storepaths.append(fn)
+                else:
+                    detail, fn = detail if isinstance(detail, tuple) else (None, detail)
+                fns.append(fn)
+            except Exception as e:
+                if on_error == "raise":
+                    raise
+                if on_error == "return":
+                    out[p] = e
+                paths.remove(p)
+
         if getpaths:
             self.fs.get(getpaths, storepaths)
             self.save_cache()
 
-        out = {}
         callback.set_size(len(paths))
         for p, fn in zip(paths, fns):
             with open(fn, "rb") as f:

fsspec/implementations/tests/test_cached.py

@@ -924,3 +924,28 @@ def test_str():
     lfs = LocalFileSystem()
     cfs = CachingFileSystem(fs=lfs)
     assert "CachingFileSystem" in str(cfs)
+
+
+def test_getitems_errors(tmpdir):
+    tmpdir = str(tmpdir)
+    os.makedirs(os.path.join(tmpdir, "afolder"))
+    open(os.path.join(tmpdir, "afile"), "w").write("test")
+    open(os.path.join(tmpdir, "afolder", "anotherfile"), "w").write("test2")
+    m = fsspec.get_mapper("file://" + tmpdir)
+    assert m.getitems(["afile", "bfile"], on_error="omit") == {"afile": b"test"}
+
+    # my code
+    m2 = fsspec.get_mapper("simplecache::file://" + tmpdir)
+    assert m2.getitems(["afile"], on_error="omit") == {"afile": b"test"}  # works
+    assert m2.getitems(["afile", "bfile"], on_error="omit") == {
+        "afile": b"test"
+    }  # throws KeyError
+
+    with pytest.raises(KeyError):
+        m.getitems(["afile", "bfile"])
+    out = m.getitems(["afile", "bfile"], on_error="return")
+    assert isinstance(out["bfile"], KeyError)
+    m = fsspec.get_mapper("file://" + tmpdir, missing_exceptions=())
+    assert m.getitems(["afile", "bfile"], on_error="omit") == {"afile": b"test"}
+    with pytest.raises(FileNotFoundError):
+        m.getitems(["afile", "bfile"])