mirror of
https://github.com/Stone-Red-Code/dvc.git
synced 2026-11-02 07:18:25 +01:00
objects.fs: do not use format_link, DvcException and fix_env from DVC
* Introduce ConfigError and AuthError in dvc.objects.fs and raise it instead of dvc.config.ConfigError. * Provide hints and links by catching those errors in dvc.cli.main, i.e decouple UI messages from filesystems for AuthError and ConfigError. * Copy flatten/unflatten utils to dvc.objects.fs.utils.
This commit is contained in:
committed by
Ruslan Kuprieiev
parent
48176b80f6
commit
3872f79dad
+13
-4
@@ -38,8 +38,7 @@ 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.objects.fs.base import RemoteMissingDepsError
|
||||
from dvc.fs import AuthError, ConfigError, RemoteMissingDepsError
|
||||
from dvc.utils import error_link, format_link
|
||||
|
||||
if isinstance(exc, RemoteMissingDepsError):
|
||||
@@ -72,6 +71,17 @@ def _log_exceptions(exc: Exception):
|
||||
)
|
||||
return
|
||||
|
||||
if isinstance(exc, (AuthError, ConfigError)):
|
||||
link = format_link("https://man.dvc.org/remote/modify")
|
||||
logger.exception("configuration error")
|
||||
logger.exception(
|
||||
f"{exc!s}\nLearn more about configuration settings at {link}.",
|
||||
extra={"tb_only": True},
|
||||
)
|
||||
return 251
|
||||
|
||||
from dvc.objects.cache import DiskError
|
||||
|
||||
if isinstance(exc, DiskError):
|
||||
from dvc.utils import relpath
|
||||
|
||||
@@ -172,8 +182,7 @@ def main(argv=None): # noqa: C901
|
||||
ret = 254
|
||||
except Exception as exc: # noqa, pylint: disable=broad-except
|
||||
# pylint: disable=no-member
|
||||
_log_exceptions(exc)
|
||||
ret = 255
|
||||
ret = _log_exceptions(exc) or 255
|
||||
|
||||
try:
|
||||
from dvc import analytics
|
||||
|
||||
+4
-3
@@ -24,9 +24,10 @@ from dvc.objects.fs import ( # noqa: F401
|
||||
get_fs_config,
|
||||
system,
|
||||
)
|
||||
from dvc.objects.fs.base import ( # noqa: F401
|
||||
AnyFSPath,
|
||||
FileSystem,
|
||||
from dvc.objects.fs.base import AnyFSPath, FileSystem # noqa: F401
|
||||
from dvc.objects.fs.errors import ( # noqa: F401
|
||||
AuthError,
|
||||
ConfigError,
|
||||
RemoteMissingDepsError,
|
||||
)
|
||||
from dvc.objects.fs.implementations.azure import AzureAuthError # noqa: F401
|
||||
|
||||
+1
-17
@@ -21,6 +21,7 @@ from funcy import cached_property
|
||||
|
||||
from ..executors import ThreadPoolExecutor
|
||||
from ._callback import DEFAULT_CALLBACK, FsspecCallback
|
||||
from .errors import RemoteMissingDepsError
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from typing import BinaryIO, TextIO
|
||||
@@ -51,23 +52,6 @@ class LinkError(OSError):
|
||||
)
|
||||
|
||||
|
||||
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:
|
||||
sep = "/"
|
||||
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
from typing import TYPE_CHECKING, List
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from .base import FileSystem
|
||||
|
||||
|
||||
class AuthError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
class ConfigError(Exception):
|
||||
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"
|
||||
)
|
||||
@@ -6,10 +6,8 @@ from fsspec.asyn import fsspec_loop
|
||||
from fsspec.utils import infer_storage_options
|
||||
from funcy import cached_property, memoize, wrap_prop
|
||||
|
||||
from dvc.exceptions import DvcException
|
||||
from dvc.utils import format_link
|
||||
|
||||
from ..base import ObjectFileSystem
|
||||
from ..errors import AuthError
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
_DEFAULT_CREDS_STEPS = (
|
||||
@@ -19,7 +17,7 @@ _DEFAULT_CREDS_STEPS = (
|
||||
)
|
||||
|
||||
|
||||
class AzureAuthError(DvcException):
|
||||
class AzureAuthError(AuthError):
|
||||
pass
|
||||
|
||||
|
||||
@@ -91,9 +89,7 @@ class AzureFileSystem(ObjectFileSystem):
|
||||
if not (login_info["account_name"] or login_info["connection_string"]):
|
||||
raise AzureAuthError(
|
||||
"Authentication to Azure Blob Storage requires either "
|
||||
"account_name or connection_string.\nLearn more about "
|
||||
"configuration settings at "
|
||||
+ format_link("https://man.dvc.org/remote/modify")
|
||||
"account_name or connection_string."
|
||||
)
|
||||
|
||||
any_secondary = any(
|
||||
@@ -154,6 +150,5 @@ class AzureFileSystem(ObjectFileSystem):
|
||||
except (ValueError, AzureError) as e:
|
||||
raise AzureAuthError(
|
||||
f"Authentication to Azure Blob Storage via {self.login_method}"
|
||||
" failed.\nLearn more about configuration settings at"
|
||||
f" {format_link('https://man.dvc.org/remote/modify')}"
|
||||
" failed."
|
||||
) from e
|
||||
|
||||
@@ -4,23 +4,21 @@ import threading
|
||||
|
||||
from funcy import cached_property, wrap_prop
|
||||
|
||||
from dvc.exceptions import DvcException
|
||||
from dvc.utils import format_link
|
||||
|
||||
from ..base import FileSystem
|
||||
from ..errors import AuthError, ConfigError
|
||||
from ..utils import tmp_fname
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
FOLDER_MIME_TYPE = "application/vnd.google-apps.folder"
|
||||
|
||||
|
||||
class GDriveAuthError(DvcException):
|
||||
class GDriveAuthError(AuthError):
|
||||
def __init__(self, cred_location):
|
||||
|
||||
if cred_location:
|
||||
message = (
|
||||
"GDrive remote auth failed with credentials in '{}'.\n"
|
||||
"Backup first, remove or fix them, and run DVC again.\n"
|
||||
"Backup first, remove or fix them, and run again.\n"
|
||||
"It should do auth again and refresh the credentials.\n\n"
|
||||
"Details:".format(cred_location)
|
||||
)
|
||||
@@ -52,12 +50,7 @@ class GDriveFileSystem(FileSystem): # pylint:disable=abstract-method
|
||||
opts = infer_storage_options(self.url)
|
||||
|
||||
if not opts["host"]:
|
||||
raise DvcException(
|
||||
"Empty GDrive URL '{}'. Learn more at {}".format(
|
||||
config["url"],
|
||||
format_link("https://man.dvc.org/remote/add"),
|
||||
)
|
||||
)
|
||||
raise ConfigError("Empty GDrive URL '{}'.".format(config["url"]))
|
||||
|
||||
self._bucket = opts["host"]
|
||||
self._path = opts["path"].lstrip("/")
|
||||
@@ -110,24 +103,18 @@ class GDriveFileSystem(FileSystem): # pylint:disable=abstract-method
|
||||
and not self._service_account_json_file_path
|
||||
and GDriveFileSystem.GDRIVE_CREDENTIALS_DATA not in os.environ
|
||||
):
|
||||
raise DvcException(
|
||||
raise ConfigError(
|
||||
"To use service account, set "
|
||||
"`gdrive_service_account_json_file_path`, and optionally"
|
||||
"`gdrive_service_account_user_email` in DVC config\n"
|
||||
"Learn more at {}".format(
|
||||
format_link("https://man.dvc.org/remote/modify")
|
||||
)
|
||||
"`gdrive_service_account_user_email` in DVC config."
|
||||
)
|
||||
|
||||
# Validate OAuth 2.0 Client ID configuration
|
||||
if not self._use_service_account:
|
||||
if bool(self._client_id) != bool(self._client_secret):
|
||||
raise DvcException(
|
||||
raise ConfigError(
|
||||
"Please specify GDrive's client ID and secret in "
|
||||
"DVC config or omit both to use the defaults.\n"
|
||||
"Learn more at {}".format(
|
||||
format_link("https://man.dvc.org/remote/modify")
|
||||
)
|
||||
"DVC config or omit both to use the defaults."
|
||||
)
|
||||
|
||||
@cached_property
|
||||
@@ -163,10 +150,7 @@ class GDriveFileSystem(FileSystem): # pylint:disable=abstract-method
|
||||
"Client ID and secret configured do not match the "
|
||||
"actual ones used\nto access the remote. Do you "
|
||||
"use multiple GDrive remotes and forgot to\nset "
|
||||
"`gdrive_user_credentials_file` for one or more of them? "
|
||||
"Learn more at\n{}.\n".format(
|
||||
format_link("https://man.dvc.org/remote/modify")
|
||||
)
|
||||
"`gdrive_user_credentials_file` for one or more of them?"
|
||||
)
|
||||
|
||||
@wrap_prop(threading.RLock())
|
||||
|
||||
@@ -1,17 +1,28 @@
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
|
||||
from funcy import cached_property, wrap_prop
|
||||
|
||||
from dvc.utils import fix_env
|
||||
|
||||
from ..base import FileSystem
|
||||
|
||||
CHECKSUM_REGEX = re.compile(r".*\t.*\t(?P<checksum>.*)")
|
||||
|
||||
|
||||
def fix_env():
|
||||
env = os.environ.copy()
|
||||
if getattr(sys, "frozen", False):
|
||||
lp_key = "LD_LIBRARY_PATH"
|
||||
lp_orig = env.get(lp_key + "_ORIG", None)
|
||||
if lp_orig is not None:
|
||||
env[lp_key] = lp_orig
|
||||
else:
|
||||
env.pop(lp_key, None)
|
||||
return env
|
||||
|
||||
|
||||
# pylint: disable=abstract-method
|
||||
class HDFSFileSystem(FileSystem):
|
||||
protocol = "hdfs"
|
||||
@@ -58,7 +69,7 @@ class HDFSFileSystem(FileSystem):
|
||||
|
||||
result = self._run_command(
|
||||
f"checksum {url}",
|
||||
env=fix_env(os.environ),
|
||||
env=fix_env(),
|
||||
user=self.fs_args.get("user"),
|
||||
)
|
||||
if result is None:
|
||||
|
||||
@@ -6,6 +6,7 @@ from funcy import cached_property, memoize, wrap_with
|
||||
|
||||
from .._callback import DEFAULT_CALLBACK, FsspecCallback
|
||||
from ..base import AnyFSPath, FileSystem
|
||||
from ..errors import ConfigError
|
||||
|
||||
|
||||
@wrap_with(threading.Lock())
|
||||
@@ -42,8 +43,6 @@ class HTTPFileSystem(FileSystem):
|
||||
import aiohttp
|
||||
from fsspec.asyn import fsspec_loop
|
||||
|
||||
from dvc.config import ConfigError
|
||||
|
||||
credentials = {}
|
||||
client_kwargs = credentials.setdefault("client_kwargs", {})
|
||||
|
||||
|
||||
@@ -5,6 +5,8 @@ from collections import defaultdict
|
||||
from funcy import cached_property, wrap_prop
|
||||
|
||||
from ..base import ObjectFileSystem
|
||||
from ..errors import ConfigError
|
||||
from ..utils import flatten, unflatten
|
||||
|
||||
_AWS_CONFIG_PATH = os.path.join(os.path.expanduser("~"), ".aws", "config")
|
||||
|
||||
@@ -72,9 +74,6 @@ class S3FileSystem(ObjectFileSystem):
|
||||
return self._split_s3_config(s3_config)
|
||||
|
||||
def _prepare_credentials(self, **config):
|
||||
from dvc.config import ConfigError
|
||||
from dvc.utils.flatten import flatten, unflatten
|
||||
|
||||
login_info = defaultdict(dict)
|
||||
|
||||
# credentials
|
||||
|
||||
@@ -236,3 +236,15 @@ def human_readable_to_bytes(value: str) -> int:
|
||||
|
||||
multiplier = MULTIPLIERS.get(suffix, 1)
|
||||
return int(value) * multiplier
|
||||
|
||||
|
||||
def flatten(d):
|
||||
import flatten_dict
|
||||
|
||||
return flatten_dict.flatten(d, reducer="dot")
|
||||
|
||||
|
||||
def unflatten(d):
|
||||
import flatten_dict
|
||||
|
||||
return flatten_dict.unflatten(d, splitter="dot")
|
||||
|
||||
@@ -2,8 +2,7 @@ import os
|
||||
|
||||
import pytest
|
||||
|
||||
from dvc.config import ConfigError
|
||||
from dvc.fs import S3FileSystem
|
||||
from dvc.fs import ConfigError, S3FileSystem
|
||||
|
||||
bucket_name = "bucket-name"
|
||||
prefix = "some/prefix"
|
||||
|
||||
Reference in New Issue
Block a user