fs: walk: get rid of onerror

Makes dvc/repofs and other filesystems comply with fsspec.

dvc/fs/base.py

@@ -154,7 +154,7 @@ class FileSystem:
         """Check if this file is an independent copy."""
         return False  # We can't be sure by default
 
-    def walk(self, top, topdown=True, onerror=None, **kwargs):
+    def walk(self, top, topdown=True, **kwargs):
         """Return a generator with (root, dirs, files)."""
         raise NotImplementedError
 

dvc/fs/dvc.py

@@ -132,18 +132,14 @@ class DvcFileSystem(FileSystem):  # pylint:disable=abstract-method
         for dname in dirs:
             yield from self._walk(self.path.join(root, dname))
 
-    def walk(self, top, topdown=True, onerror=None, **kwargs):
+    def walk(self, top, topdown=True, **kwargs):
         assert topdown
         try:
             info = self.info(top)
         except FileNotFoundError:
-            if onerror is not None:
-                onerror(FileNotFoundError(top))
             return
 
         if info["type"] != "directory":
-            if onerror is not None:
-                onerror(NotADirectoryError(top))
             return
 
         yield from self._walk(top, topdown=topdown, **kwargs)

dvc/fs/fsspec_wrapper.py

@@ -1,15 +1,6 @@
 import os
 import shutil
-from typing import (
-    IO,
-    TYPE_CHECKING,
-    Any,
-    Callable,
-    Dict,
-    Iterator,
-    Optional,
-    overload,
-)
+from typing import IO, TYPE_CHECKING, Any, Dict, Iterator, Optional, overload
 
 from funcy import cached_property
 from tqdm.utils import CallbackIOWrapper
@@ -149,10 +140,9 @@ class FSSpecWrapper(FileSystem):
         self,
         top: AnyFSPath,
         topdown: bool = True,
-        onerror: Callable[[OSError], None] = None,
         **kwargs: Any,
     ):
-        return self.fs.walk(top, topdown=topdown, onerror=onerror, **kwargs)
+        return self.fs.walk(top, topdown=topdown, **kwargs)
 
 
 # pylint: disable=abstract-method

dvc/fs/local.py

@@ -51,14 +51,15 @@ class LocalFileSystem(FileSystem):
     def iscopy(self, path):
         return not (System.is_symlink(path) or System.is_hardlink(path))
 
-    def walk(self, top, topdown=True, onerror=None, **kwargs):
+    def walk(self, top, topdown=True, **kwargs):
         """Directory fs generator.
 
         See `os.walk` for the docs. Differences:
         - no support for symlinks
         """
         for root, dirs, files in os.walk(
-            top, topdown=topdown, onerror=onerror
+            top,
+            topdown=topdown,
         ):
             yield os.path.normpath(root), dirs, files
 

dvc/fs/repo.py

@@ -404,14 +404,12 @@ class RepoFileSystem(FileSystem):  # pylint:disable=abstract-method
             elif dirname in repo_set:
                 yield from self._walk(repo_walk, None, dvcfiles=dvcfiles)
 
-    def walk(self, top, topdown=True, onerror=None, **kwargs):
+    def walk(self, top, topdown=True, **kwargs):
         """Walk and merge both DVC and repo fss.
 
         Args:
             top: path to walk from
             topdown: if True, fs will be walked from top down.
-            onerror: if set, onerror function will be called if an error
-                occurs (by default errors are ignored).
             dvcfiles: if True, dvcfiles will be included in the files list
                 for walked directories.
 
@@ -421,13 +419,9 @@ class RepoFileSystem(FileSystem):  # pylint:disable=abstract-method
         assert topdown
 
         if not self.exists(top):
-            if onerror is not None:
-                onerror(FileNotFoundError(top))
             return
 
         if not self.isdir(top):
-            if onerror is not None:
-                onerror(NotADirectoryError(top))
             return
 
         repo = self._get_repo(os.path.abspath(top))
@@ -436,24 +430,18 @@ class RepoFileSystem(FileSystem):  # pylint:disable=abstract-method
         fs, dvc_fs, dvc_path = self._get_fs_pair(top)
         repo_exists = fs.exists(top)
 
-        repo_walk = repo.dvcignore.walk(
-            fs, top, topdown=topdown, onerror=onerror, **kwargs
-        )
+        repo_walk = repo.dvcignore.walk(fs, top, topdown=topdown, **kwargs)
 
         if not dvc_fs or (repo_exists and dvc_fs.isdvc(dvc_path)):
             yield from self._walk(repo_walk, None, dvcfiles=dvcfiles)
             return
 
         if not repo_exists:
-            yield from _wrap_walk(
-                dvc_fs, dvc_path, topdown=topdown, onerror=onerror, **kwargs
-            )
+            yield from _wrap_walk(dvc_fs, dvc_path, topdown=topdown, **kwargs)
 
         dvc_walk = None
         if dvc_fs.exists(dvc_path):
-            dvc_walk = _wrap_walk(
-                dvc_fs, dvc_path, topdown=topdown, onerror=onerror, **kwargs
-            )
+            dvc_walk = _wrap_walk(dvc_fs, dvc_path, topdown=topdown, **kwargs)
 
         yield from self._walk(repo_walk, dvc_walk, dvcfiles=dvcfiles)
 

dvc/repo/ls.py

@@ -47,23 +47,16 @@ def ls(url, path=None, rev=None, recursive=None, dvc_only=False):
 
 
 def _ls(repo, fs_path, recursive=None, dvc_only=False):
-    def onerror(exc):
-        raise exc
-
     fs = repo.repo_fs
     infos = []
-    try:
-        for root, dirs, files in fs.walk(
-            fs_path, onerror=onerror, dvcfiles=True
-        ):
-            entries = chain(files, dirs) if not recursive else files
-            infos.extend(fs.path.join(root, entry) for entry in entries)
-            if not recursive:
-                break
-    except NotADirectoryError:
+    for root, dirs, files in fs.walk(fs_path, dvcfiles=True):
+        entries = chain(files, dirs) if not recursive else files
+        infos.extend(fs.path.join(root, entry) for entry in entries)
+        if not recursive:
+            break
+
+    if not infos and fs.isfile(fs_path):
         infos.append(fs_path)
-    except FileNotFoundError:
-        return {}
 
     ret = {}
     for info in infos:

tests/unit/fs/test_dvc.py

@@ -181,26 +181,19 @@ def test_walk_dir(tmp_dir, dvc):
     assert len(actual) == len(expected)
 
 
-def test_walk_onerror(tmp_dir, dvc):
-    def onerror(exc):
-        raise exc
+def test_walk_missing(tmp_dir, dvc):
+    fs = DvcFileSystem(repo=dvc)
 
+    for _ in fs.walk("dir"):
+        pass
+
+
+def test_walk_not_a_dir(tmp_dir, dvc):
     tmp_dir.dvc_gen("foo", "foo")
     fs = DvcFileSystem(repo=dvc)
 
-    # path does not exist
-    for _ in fs.walk("dir"):
-        pass
-    with pytest.raises(OSError):
-        for _ in fs.walk("dir", onerror=onerror):
-            pass
-
-    # path is not a directory
     for _ in fs.walk("foo"):
         pass
-    with pytest.raises(OSError):
-        for _ in fs.walk("foo", onerror=onerror):
-            pass
 
 
 def test_isdvc(tmp_dir, dvc):

tests/unit/fs/test_repo.py

@@ -291,26 +291,19 @@ def test_walk_mixed_dir(tmp_dir, scm, dvc):
     assert len(actual) == len(expected)
 
 
-def test_walk_onerror(tmp_dir, dvc):
-    def onerror(exc):
-        raise exc
+def test_walk_missing(tmp_dir, dvc):
+    fs = RepoFileSystem(repo=dvc)
 
+    for _ in fs.walk("dir"):
+        pass
+
+
+def test_walk_not_a_dir(tmp_dir, dvc):
     tmp_dir.dvc_gen("foo", "foo")
     fs = RepoFileSystem(repo=dvc)
 
-    # path does not exist
-    for _ in fs.walk("dir"):
-        pass
-    with pytest.raises(OSError):
-        for _ in fs.walk("dir", onerror=onerror):
-            pass
-
-    # path is not a directory
     for _ in fs.walk("foo"):
         pass
-    with pytest.raises(OSError):
-        for _ in fs.walk("foo", onerror=onerror):
-            pass
 
 
 def test_isdvc(tmp_dir, dvc):