diff --git a/dvc/cli/__init__.py b/dvc/cli/__init__.py index 2a5cb9ce7..5c73acd21 100644 --- a/dvc/cli/__init__.py +++ b/dvc/cli/__init__.py @@ -39,7 +39,38 @@ def parse_args(argv=None): def _log_exceptions(exc: Exception): """Try to log some known exceptions, that are not DVCExceptions.""" from dvc.objects.cache import DiskError - from dvc.utils import error_link + from dvc.objects.fs.base import RemoteMissingDepsError + from dvc.utils import error_link, format_link + + if isinstance(exc, RemoteMissingDepsError): + from dvc.utils.pkg import PKG + + proto = exc.protocol + by_pkg = { + "pip": f"pip install 'dvc[{proto}]'", + "conda": f"conda install -c conda-forge dvc-{proto}", + } + + cmd = by_pkg.get(PKG) + if cmd: + link = format_link("https://dvc.org/doc/install") + hint = ( + f"To install dvc with those dependencies, run:\n" + "\n" + f"\t{cmd}\n" + "\n" + f"See {link} for more info." + ) + else: + link = format_link("https://github.com/iterative/dvc/issues") + hint = f"\nPlease report this bug to {link}. Thank you!" + + logger.exception( + f"URL '{exc.url}' is supported but requires these missing " + f"dependencies: {exc.missing_deps}. {hint}", + extra={"tb_only": True}, + ) + return if isinstance(exc, DiskError): from dvc.utils import relpath diff --git a/dvc/objects/fs/base.py b/dvc/objects/fs/base.py index dac86e4a7..1688029b6 100644 --- a/dvc/objects/fs/base.py +++ b/dvc/objects/fs/base.py @@ -19,8 +19,6 @@ from typing import ( from funcy import cached_property -from dvc.exceptions import DvcException - from ..executors import ThreadPoolExecutor from ._callback import DEFAULT_CALLBACK, FsspecCallback @@ -42,14 +40,32 @@ AnyFSPath = str Entry = Dict[str, Any] -class RemoteActionNotImplemented(DvcException): - def __init__(self, action, scheme): - m = f"{action} is not supported for {scheme} remotes" - super().__init__(m) +class LinkError(OSError): + def __init__(self, link: str, fs: "FileSystem", path: str) -> None: + import errno + + super().__init__( + errno.EPERM, + f"{link} is not supported for {fs.protocol} by {type(fs)}", + path, + ) -class RemoteMissingDepsError(DvcException): - pass +class RemoteMissingDepsError(Exception): + def __init__( + self, + fs: "FileSystem", + protocol: str, + url: str, + missing: List[str] = None, + ) -> None: + self.protocol = protocol + self.fs = fs + self.url = url + self.missing_deps = missing or [] + super().__init__( + f"filesystem for '{protocol}': '{type(fs)}' missing dependencies" + ) class FileSystem: @@ -131,44 +147,18 @@ class FileSystem: return missing def _check_requires(self, **kwargs): - from dvc.utils import format_link - from dvc.utils.pkg import PKG - from .scheme import Schemes missing = self.get_missing_deps() if not missing: return + proto = self.protocol + if proto == Schemes.WEBDAVS: + proto = Schemes.WEBDAV + url = kwargs.get("url", f"{self.protocol}://") - - scheme = self.protocol - if scheme == Schemes.WEBDAVS: - scheme = Schemes.WEBDAV - - by_pkg = { - "pip": f"pip install 'dvc[{scheme}]'", - "conda": f"conda install -c conda-forge dvc-{scheme}", - } - - cmd = by_pkg.get(PKG) - if cmd: - link = format_link("https://dvc.org/doc/install") - hint = ( - f"To install dvc with those dependencies, run:\n" - "\n" - f"\t{cmd}\n" - "\n" - f"See {link} for more info." - ) - else: - link = format_link("https://github.com/iterative/dvc/issues") - hint = f"Please report this bug to {link}. Thank you!" - - raise RemoteMissingDepsError( - f"URL '{url}' is supported but requires these missing " - f"dependencies: {missing}. {hint}" - ) + raise RemoteMissingDepsError(self, proto, url, missing) def isdir(self, path: AnyFSPath) -> bool: return self.fs.isdir(path) @@ -295,19 +285,19 @@ class FileSystem: try: return self.fs.symlink(from_info, to_info) except AttributeError: - raise RemoteActionNotImplemented("symlink", self.protocol) + raise LinkError("symlink", self, from_info) def hardlink(self, from_info: AnyFSPath, to_info: AnyFSPath) -> None: try: return self.fs.hardlink(from_info, to_info) except AttributeError: - raise RemoteActionNotImplemented("hardlink", self.protocol) + raise LinkError("symlink", self, from_info) def reflink(self, from_info: AnyFSPath, to_info: AnyFSPath) -> None: try: return self.fs.reflink(from_info, to_info) except AttributeError: - raise RemoteActionNotImplemented("reflink", self.protocol) + raise LinkError("symlink", self, from_info) def is_symlink(self, path: AnyFSPath) -> bool: try: diff --git a/dvc/objects/fs/generic.py b/dvc/objects/fs/generic.py index 6234ec586..af4e89f9e 100644 --- a/dvc/objects/fs/generic.py +++ b/dvc/objects/fs/generic.py @@ -4,7 +4,6 @@ import os from typing import TYPE_CHECKING, List, Optional from ._callback import DEFAULT_CALLBACK -from .base import RemoteActionNotImplemented from .implementations.local import LocalFileSystem from .utils import as_atomic @@ -25,15 +24,8 @@ def _link( if not isinstance(from_fs, type(to_fs)): raise OSError(errno.EXDEV, "can't link across filesystems") - try: - func = getattr(to_fs, link) - func(from_path, to_path) - except (OSError, AttributeError, RemoteActionNotImplemented) as exc: - # NOTE: there are so many potential error codes that a link call can - # throw, that is safer to just catch them all and try another link. - raise OSError( - errno.ENOTSUP, f"'{link}' is not supported by {type(from_fs)}" - ) from exc + func = getattr(to_fs, link) + func(from_path, to_path) def copy( @@ -75,7 +67,7 @@ def _try_links( try: return _link(link, from_fs, from_path, to_fs, to_path) except OSError as exc: - if exc.errno not in [errno.ENOTSUP, errno.EXDEV]: + if exc.errno not in [errno.EPERM, errno.ENOTSUP, errno.EXDEV]: raise error = exc diff --git a/dvc/output.py b/dvc/output.py index 1b2e4d76b..54179cdfc 100644 --- a/dvc/output.py +++ b/dvc/output.py @@ -893,7 +893,7 @@ class Output: try: self.get_dir_cache(jobs=jobs, remote=remote) - except RemoteMissingDepsError: + except RemoteMissingDepsError: # pylint: disable=try-except-raise raise except DvcException: logger.debug(f"failed to pull cache for '{self}'") diff --git a/tests/unit/cli/test_main.py b/tests/unit/cli/test_main.py index 05fe48d65..95e19292d 100644 --- a/tests/unit/cli/test_main.py +++ b/tests/unit/cli/test_main.py @@ -1,9 +1,11 @@ from argparse import Namespace +import pytest from funcy import raiser from dvc.cli import main from dvc.objects.cache import DiskError +from dvc.objects.fs.base import FileSystem, RemoteMissingDepsError def test_state_pickle_errors_are_correctly_raised(tmp_dir, caplog, mocker): @@ -24,3 +26,45 @@ def test_state_pickle_errors_are_correctly_raised(tmp_dir, caplog, mocker): "and then retry this command.\n" "See for more information." ) in caplog.text + + +@pytest.mark.parametrize( + "pkg, msg", + [ + (None, "Please report this bug to"), + ("pip", "pip install 'dvc[proto]'"), + ("conda", "conda install -c conda-forge dvc-proto"), + ], +) +def test_remote_missing_deps_are_correctly_reported( + tmp_dir, caplog, mocker, pkg, msg +): + error = RemoteMissingDepsError(FileSystem(), "proto", "proto://", ["deps"]) + mocker.patch("dvc.utils.pkg.PKG", pkg) + mocker.patch( + "dvc.cli.parse_args", + return_value=Namespace( + func=raiser(error), + quiet=False, + verbose=True, + ), + ) + + assert main() == 255 + expected = ( + "URL 'proto://' is supported but requires these missing dependencies: " + "['deps']. " + ) + if pkg: + expected += ( + "To install dvc with those dependencies, run:\n\n" + f"\t{msg}\n\n" + "See for more info." + ) + else: + expected += ( + "\nPlease report this bug to " + ". " + "Thank you!" + ) + assert expected in caplog.text diff --git a/tests/unit/fs/test_base.py b/tests/unit/fs/test_base.py index d475114ba..5a04a0634 100644 --- a/tests/unit/fs/test_base.py +++ b/tests/unit/fs/test_base.py @@ -3,17 +3,8 @@ import pytest from dvc.fs import FileSystem, RemoteMissingDepsError -@pytest.mark.parametrize( - "pkg, msg", - [ - (None, "Please report this bug to"), - ("pip", "pip install"), - ("conda", "conda install"), - ], -) -def test_missing_deps(pkg, msg, mocker): +def test_missing_deps(mocker): requires = {"missing": "missing"} mocker.patch.object(FileSystem, "REQUIRES", requires) - mocker.patch("dvc.utils.pkg.PKG", pkg) - with pytest.raises(RemoteMissingDepsError, match=msg): + with pytest.raises(RemoteMissingDepsError, match="missing dependencies"): FileSystem() diff --git a/tests/unit/output/test_output.py b/tests/unit/output/test_output.py index 01b243aeb..6b93ae6d5 100644 --- a/tests/unit/output/test_output.py +++ b/tests/unit/output/test_output.py @@ -115,7 +115,7 @@ def test_remote_missing_depenency_on_dir_pull(tmp_dir, scm, dvc, mocker): with mocker.patch( "dvc.data_cloud.DataCloud.get_remote_odb", - side_effect=RemoteMissingDepsError("remote missing"), + side_effect=RemoteMissingDepsError(dvc.fs, "azure", "azure://", []), ): with pytest.raises(RemoteMissingDepsError): dvc.pull()