From 8ec1ff44debdb65c5f308eb810687b5040d66a0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saugat=20Pachhai=20=28=E0=A4=B8=E0=A5=8C=E0=A4=97=E0=A4=BE?= =?UTF-8?q?=E0=A4=A4=29?= Date: Mon, 22 Nov 2021 13:56:58 +0545 Subject: [PATCH] exceptions: move AuthError and InvalidRemote errors --- dvc/repo/experiments/executor/base.py | 11 +++++-- dvc/repo/experiments/push.py | 11 +++++-- dvc/repo/experiments/remove.py | 4 +-- dvc/repo/experiments/utils.py | 40 +++++++++++++++++++++++-- dvc/scm/base.py | 13 +++----- dvc/scm/exceptions.py | 12 ++++++++ dvc/scm/git/backend/dulwich/__init__.py | 15 +++++----- 7 files changed, 80 insertions(+), 26 deletions(-) diff --git a/dvc/repo/experiments/executor/base.py b/dvc/repo/experiments/executor/base.py index 9d8111d75..3d506cc45 100644 --- a/dvc/repo/experiments/executor/base.py +++ b/dvc/repo/experiments/executor/base.py @@ -183,9 +183,11 @@ class BaseExecutor(ABC): on_diverged: Callback in the form on_diverged(ref, is_checkpoint) to be called when an experiment ref has diverged. """ + from ..utils import iter_remote_refs + refs = [] has_checkpoint = False - for ref in dest_scm.iter_remote_refs(url, base=EXPS_NAMESPACE): + for ref in iter_remote_refs(dest_scm, url, base=EXPS_NAMESPACE): if ref == EXEC_CHECKPOINT: has_checkpoint = True elif not ref.startswith(EXEC_NAMESPACE) and ref != EXPS_STASH: @@ -222,6 +224,8 @@ class BaseExecutor(ABC): @classmethod def _validate_remotes(cls, dvc: "Repo", git_remote: Optional[str]): + from dvc.scm.base import InvalidRemoteSCMRepo + from dvc.scm.exceptions import InvalidRemote if git_remote == dvc.root_dir: logger.warning( @@ -230,7 +234,10 @@ class BaseExecutor(ABC): "will automatically be pushed to the default DVC remote " "(if any) on each experiment commit." ) - dvc.scm.validate_git_remote(git_remote) + try: + dvc.scm.validate_git_remote(git_remote) + except InvalidRemote as exc: + raise InvalidRemoteSCMRepo(str(exc)) dvc.cloud.get_remote_odb() @classmethod diff --git a/dvc/repo/experiments/push.py b/dvc/repo/experiments/push.py index d42c3736b..eec925395 100644 --- a/dvc/repo/experiments/push.py +++ b/dvc/repo/experiments/push.py @@ -4,7 +4,7 @@ from dvc.exceptions import DvcException, InvalidArgumentError from dvc.repo import locked from dvc.repo.scm_context import scm_context -from .utils import exp_commits, resolve_exp_ref +from .utils import exp_commits, push_refspec, resolve_exp_ref logger = logging.getLogger(__name__) @@ -37,8 +37,13 @@ def push( refname = str(exp_ref) logger.debug("git push experiment '%s' -> '%s'", exp_ref, git_remote) - repo.scm.push_refspec( - git_remote, refname, refname, force=force, on_diverged=on_diverged + push_refspec( + repo.scm, + git_remote, + refname, + refname, + force=force, + on_diverged=on_diverged, ) if push_cache: diff --git a/dvc/repo/experiments/remove.py b/dvc/repo/experiments/remove.py index c712572f5..4d66ca61e 100644 --- a/dvc/repo/experiments/remove.py +++ b/dvc/repo/experiments/remove.py @@ -6,7 +6,7 @@ from dvc.repo import locked from dvc.repo.scm_context import scm_context from dvc.scm.base import RevError -from .utils import exp_refs, remove_exp_refs, resolve_exp_ref +from .utils import exp_refs, push_refspec, remove_exp_refs, resolve_exp_ref logger = logging.getLogger(__name__) @@ -78,7 +78,7 @@ def _remove_commited_exps( remove_exp_refs(repo.scm, remove_list) else: for ref_info in remove_list: - repo.scm.push_refspec(remote, None, str(ref_info)) + push_refspec(repo.scm, remote, None, str(ref_info)) return remain_list diff --git a/dvc/repo/experiments/utils.py b/dvc/repo/experiments/utils.py index 1c68a2b0a..66709d626 100644 --- a/dvc/repo/experiments/utils.py +++ b/dvc/repo/experiments/utils.py @@ -1,4 +1,4 @@ -from typing import Generator, Iterable, Optional, Set +from typing import Callable, Generator, Iterable, Optional, Set from dvc.exceptions import InvalidArgumentError from dvc.scm.git import Git @@ -49,11 +49,45 @@ def exp_refs_by_baseline( yield ExpRefInfo.from_ref(ref) +def iter_remote_refs( + scm: "Git", url: str, base: Optional[str] = None, **kwargs +): + from dvc.scm.base import GitAuthError, InvalidRemoteSCMRepo + from dvc.scm.exceptions import AuthError, InvalidRemote + + try: + yield from scm.iter_remote_refs(url, base=base, **kwargs) + except InvalidRemote as exc: + raise InvalidRemoteSCMRepo(str(exc)) + except AuthError as exc: + raise GitAuthError(str(exc)) + + +def push_refspec( + scm: "Git", + url: str, + src: Optional[str], + dest: str, + force: bool = False, + on_diverged: Optional[Callable[[str, str], bool]] = None, + **kwargs, +): + from dvc.scm.base import GitAuthError + from dvc.scm.exceptions import AuthError + + try: + return scm.push_refspec( + url, src, dest, force=force, on_diverged=on_diverged, **kwargs + ) + except AuthError as exc: + raise GitAuthError(str(exc)) + + def remote_exp_refs( scm: "Git", url: str ) -> Generator["ExpRefInfo", None, None]: """Iterate over all remote experiment refs.""" - for ref in scm.iter_remote_refs(url, base=EXPS_NAMESPACE): + for ref in iter_remote_refs(scm, url, base=EXPS_NAMESPACE): if ref.startswith(EXEC_NAMESPACE) or ref == EXPS_STASH: continue yield ExpRefInfo.from_ref(ref) @@ -73,7 +107,7 @@ def remote_exp_refs_by_baseline( ) -> Generator["ExpRefInfo", None, None]: """Iterate over all remote experiment refs with the specified baseline.""" ref_info = ExpRefInfo(baseline_sha=rev) - for ref in scm.iter_remote_refs(url, base=str(ref_info)): + for ref in iter_remote_refs(scm, url, base=str(ref_info)): if ref.startswith(EXEC_NAMESPACE) or ref == EXPS_STASH: continue yield ExpRefInfo.from_ref(ref) diff --git a/dvc/scm/base.py b/dvc/scm/base.py index e29a05fb0..cfcc51d4e 100644 --- a/dvc/scm/base.py +++ b/dvc/scm/base.py @@ -29,18 +29,13 @@ class NoSCMError(SCMError): class InvalidRemoteSCMRepo(SCMError): - def __init__(self, url: str): - msg = f"'{url}' is not a valid Git remote or URL" - super().__init__(msg) + pass class GitAuthError(SCMError): - def __init__(self, url: str): - super().__init__( - f"HTTP Git authentication is not supported: '{url}'" - "\nSee https://dvc.org/doc//user-guide/" - "troubleshooting#git-auth" - ) + def __init__(self, reason: str) -> None: + doc = "See https://dvc.org/doc//user-guide/troubleshooting#git-auth" + super().__init__(f"{reason}\n{doc}") class Base: diff --git a/dvc/scm/exceptions.py b/dvc/scm/exceptions.py index cca9307a0..2ff9e0f82 100644 --- a/dvc/scm/exceptions.py +++ b/dvc/scm/exceptions.py @@ -16,3 +16,15 @@ class FileNotInRepoError(SCMError): class MergeConflictError(SCMError): pass + + +class InvalidRemote(SCMError): + def __init__(self, url: str) -> None: + self.url = url + super().__init__(f"'{url}' is not a valid Git remote or URL") + + +class AuthError(SCMError): + def __init__(self, url: str) -> None: + self.url = url + super().__init__(f"HTTP Git authentication is not supported: '{url}'") diff --git a/dvc/scm/git/backend/dulwich/__init__.py b/dvc/scm/git/backend/dulwich/__init__.py index 6a4913e2c..c479740ac 100644 --- a/dvc/scm/git/backend/dulwich/__init__.py +++ b/dvc/scm/git/backend/dulwich/__init__.py @@ -19,7 +19,8 @@ from typing import ( from funcy import cached_property from dvc.progress import Tqdm -from dvc.scm.base import GitAuthError, InvalidRemoteSCMRepo, SCMError +from dvc.scm.base import SCMError +from dvc.scm.exceptions import AuthError, InvalidRemote from dvc.utils import relpath from ...objects import GitObject @@ -370,7 +371,7 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method _remote, location = get_remote_repo(self.repo, url) client, path = get_transport_and_path(location, **kwargs) except Exception as exc: - raise InvalidRemoteSCMRepo(url) from exc + raise InvalidRemote(url) from exc try: if base: @@ -382,9 +383,9 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method else: yield from (os.fsdecode(ref) for ref in client.get_refs(path)) except NotGitRepository as exc: - raise InvalidRemoteSCMRepo(url) from exc + raise InvalidRemote(url) from exc except HTTPUnauthorized: - raise GitAuthError(url) + raise AuthError(url) def get_refs_containing(self, rev: str, pattern: Optional[str] = None): raise NotImplementedError @@ -458,7 +459,7 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method except (NotGitRepository, SendPackError) as exc: raise SCMError("Git failed to push '{src}' to '{url}'") from exc except HTTPUnauthorized: - raise GitAuthError(url) + raise AuthError(url) def _push_dest_refs( self, src: Optional[str], dest: str @@ -676,11 +677,11 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method _, location = get_remote_repo(self.repo, url) client, path = get_transport_and_path(location, **kwargs) except Exception as exc: - raise InvalidRemoteSCMRepo(url) from exc + raise InvalidRemote(url) from exc if isinstance(client, LocalGitClient) and not os.path.exists( os.path.join("", path) ): - raise InvalidRemoteSCMRepo(url) + raise InvalidRemote(url) def check_ref_format(self, refname: str): from dulwich.refs import check_ref_format