From aed04e7cc9eb62b98a1682b7a869253e5f9fa07a Mon Sep 17 00:00:00 2001 From: "Mr. Outis" Date: Thu, 2 Jan 2020 16:36:25 -0600 Subject: [PATCH 1/2] exceptions: remove noop code --- dvc/exceptions.py | 18 ++---------------- dvc/remote/gdrive.py | 11 +++-------- dvc/stage.py | 3 +-- 3 files changed, 6 insertions(+), 26 deletions(-) diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 8192a674d..f67a01751 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -13,18 +13,6 @@ class DvcException(Exception): cause (Exception): optional cause exception. """ - def __init__(self, msg, cause=None): - # NOTE: unlike python 3, python 2 doesn't have built-in support - # for chained exceptions, so we are using our own implementation. - self.cause = cause - self.cause_tb = None - if cause: - try: - self.cause_tb = traceback.format_exc() - except AttributeError: # pragma: no cover - pass - super().__init__(msg) - class OutputDuplicationError(DvcException): """Thrown if a file/directory is specified as an output in more than one @@ -109,8 +97,7 @@ class ArgumentDuplicationError(DvcException): def __init__(self, path): assert isinstance(path, str) - msg = "file '{}' is specified more than once." - super().__init__(msg.format(path)) + super().__init__("file '{}' is specified more than once.".format(path)) class MoveNotDataSourceError(DvcException): @@ -175,8 +162,7 @@ class ConfirmRemoveError(DvcException): class InitError(DvcException): - def __init__(self, msg): - super().__init__(msg) + pass class ReproductionError(DvcException): diff --git a/dvc/remote/gdrive.py b/dvc/remote/gdrive.py index a23f9f17f..7167bb132 100644 --- a/dvc/remote/gdrive.py +++ b/dvc/remote/gdrive.py @@ -19,18 +19,13 @@ FOLDER_MIME_TYPE = "application/vnd.google-apps.folder" class GDriveRetriableError(DvcException): - def __init__(self, msg, cause=None): - super().__init__(msg, cause=cause) - + pass class GDriveAccessTokenRefreshError(DvcException): - def __init__(self, msg, cause=None): - super().__init__(msg, cause=cause) - + pass class GDriveMissedCredentialKeyError(DvcException): - def __init__(self, msg, cause=None): - super().__init__(msg, cause=cause) + pass @decorator diff --git a/dvc/stage.py b/dvc/stage.py index 4633cd67b..176da2792 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -71,8 +71,7 @@ class StageFileIsNotDvcFileError(DvcException): class StageFileBadNameError(DvcException): - def __init__(self, msg): - super().__init__(msg) + pass class StagePathOutsideError(DvcException): From 5611ccb43a1e2465a1d4d7e4a376c06f6fd83385 Mon Sep 17 00:00:00 2001 From: "Mr. Outis" Date: Thu, 2 Jan 2020 17:02:46 -0600 Subject: [PATCH 2/2] exceptions: remove cause --- dvc/api.py | 7 +++---- dvc/config.py | 17 ++++++----------- dvc/exceptions.py | 30 +++++++++--------------------- dvc/logger.py | 6 +++--- dvc/remote/base.py | 7 +++---- dvc/remote/gdrive.py | 21 ++++++++++++--------- dvc/remote/s3.py | 4 ++-- dvc/remote/ssh/connection.py | 14 ++++++-------- dvc/repo/__init__.py | 10 +++++----- dvc/repo/checkout.py | 2 +- dvc/repo/reproduce.py | 4 ++-- dvc/rwlock.py | 13 ++++++------- dvc/scm/base.py | 11 ++++------- dvc/scm/git/__init__.py | 8 ++++---- dvc/scm/git/tree.py | 5 ++--- dvc/system.py | 6 +++--- dvc/utils/stage.py | 4 ++-- tests/func/test_checkout.py | 8 ++++---- tests/unit/test_logger.py | 12 +++++------- 19 files changed, 82 insertions(+), 107 deletions(-) diff --git a/dvc/api.py b/dvc/api.py index b6a6b2cdd..ffb044a02 100644 --- a/dvc/api.py +++ b/dvc/api.py @@ -104,9 +104,8 @@ def summon(name, repo=None, rev=None, summon_file="dvcsummon.yaml", args=None): info = obj["summon"] except SummonError as exc: raise SummonError( - str(exc) + " at '{}' in '{}'".format(summon_file, repo), - cause=exc.cause, - ) + str(exc) + " at '{}' in '{}'".format(summon_file, repo) + ) from exc _pull_dependencies(_repo, info.get("deps", [])) @@ -138,7 +137,7 @@ def _get_object_from_summon_file(name, path): except FileMissingError: raise SummonError("Summon file not found") except ruamel.yaml.YAMLError as exc: - raise SummonError("Failed to parse summon file", exc) + raise SummonError("Failed to parse summon file") from exc except Invalid as exc: raise SummonError(str(exc)) diff --git a/dvc/config.py b/dvc/config.py index cc2fbdb4d..61ceb6756 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -17,19 +17,14 @@ logger = logging.getLogger(__name__) class ConfigError(DvcException): - """DVC config exception. + """DVC config exception.""" - Args: - msg (str): error message. - cause (Exception): optional exception that has caused this error. - """ - - def __init__(self, msg, cause=None): - super().__init__("config file error: {}".format(msg), cause=cause) + def __init__(self, msg): + super().__init__("config file error: {}".format(msg)) class NoRemoteError(ConfigError): - def __init__(self, command, cause=None): + def __init__(self, command): msg = ( "no remote specified. Setup default remote with\n" " dvc config core.remote \n" @@ -37,7 +32,7 @@ class NoRemoteError(ConfigError): " dvc {} -r \n".format(command) ) - super().__init__(msg, cause=cause) + super().__init__(msg) def supported_cache_type(types): @@ -392,7 +387,7 @@ class Config(object): # pylint: disable=too-many-instance-attributes try: d = self.COMPILED_SCHEMA(d) except Invalid as exc: - raise ConfigError(str(exc), cause=exc) + raise ConfigError(str(exc)) from exc self.config = configobj.ConfigObj(d, write_empty_values=True) def save(self, config=None): diff --git a/dvc/exceptions.py b/dvc/exceptions.py index f67a01751..e6cd70764 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -1,17 +1,10 @@ """Exceptions raised by the dvc.""" -import traceback - from dvc.utils import relpath class DvcException(Exception): - """Base class for all dvc exceptions. - - Args: - msg (unicode): message for this exception. - cause (Exception): optional cause exception. - """ + """Base class for all dvc exceptions.""" class OutputDuplicationError(DvcException): @@ -166,10 +159,9 @@ class InitError(DvcException): class ReproductionError(DvcException): - def __init__(self, dvc_file_name, ex): + def __init__(self, dvc_file_name): self.path = dvc_file_name - msg = "failed to reproduce '{}'".format(dvc_file_name) - super().__init__(msg, cause=ex) + super().__init__("failed to reproduce '{}'".format(dvc_file_name)) class BadMetricError(DvcException): @@ -191,12 +183,11 @@ class NoMetricsError(DvcException): class StageFileCorruptedError(DvcException): - def __init__(self, path, cause=None): + def __init__(self, path): path = relpath(path) super().__init__( "unable to read DVC-file: {} " - "YAML file structure is corrupted".format(path), - cause=cause, + "YAML file structure is corrupted".format(path) ) @@ -222,10 +213,8 @@ class OverlappingOutputPathsError(DvcException): class CheckoutErrorSuggestGit(DvcException): - def __init__(self, target, cause): - super().__init__( - "Did you mean `git checkout {}`?".format(target), cause=cause - ) + def __init__(self, target): + super().__init__("Did you mean `git checkout {}`?".format(target)) class ETagMismatchError(DvcException): @@ -237,10 +226,9 @@ class ETagMismatchError(DvcException): class FileMissingError(DvcException): - def __init__(self, path, cause=None): + def __init__(self, path): super().__init__( - "Can't find '{}' neither locally nor on remote".format(path), - cause=cause, + "Can't find '{}' neither locally nor on remote".format(path) ) diff --git a/dvc/logger.py b/dvc/logger.py index abe5757b9..95362d19c 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -114,11 +114,11 @@ class ColorFormatter(logging.Formatter): tb_list = [tb] # NOTE: parsing chained exceptions. See dvc/exceptions.py for more info - while hasattr(exc, "cause") and exc.cause: - exc_list.append(str(exc.cause)) + while hasattr(exc, "__cause__") and exc.__cause__: + exc_list.append(str(exc.__cause__)) if hasattr(exc, "cause_tb") and exc.cause_tb: tb_list.insert(0, str(exc.cause_tb)) - exc = exc.cause + exc = exc.__cause__ return exc_list, tb_list diff --git a/dvc/remote/base.py b/dvc/remote/base.py index df71c9819..32e14069e 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -63,10 +63,9 @@ class RemoteMissingDepsError(DvcException): class DirCacheError(DvcException): - def __init__(self, checksum, cause=None): + def __init__(self, checksum): super().__init__( - "Failed to load dir cache for checksum: '{}'.".format(checksum), - cause=cause, + "Failed to load dir cache for checksum: '{}'.".format(checksum) ) @@ -264,7 +263,7 @@ class RemoteBASE(object): with self.cache.open(path_info, "r") as fobj: d = json.load(fobj) except (ValueError, FileNotFoundError) as exc: - raise DirCacheError(checksum, cause=exc) + raise DirCacheError(checksum) from exc if not isinstance(d, list): msg = "dir cache file format error '{}' [skipping the file]" diff --git a/dvc/remote/gdrive.py b/dvc/remote/gdrive.py index 7167bb132..704938e91 100644 --- a/dvc/remote/gdrive.py +++ b/dvc/remote/gdrive.py @@ -21,9 +21,11 @@ FOLDER_MIME_TYPE = "application/vnd.google-apps.folder" class GDriveRetriableError(DvcException): pass + class GDriveAccessTokenRefreshError(DvcException): pass + class GDriveMissedCredentialKeyError(DvcException): pass @@ -203,21 +205,22 @@ class RemoteGDrive(RemoteBASE): try: gauth.CommandLineAuth() - except RefreshError as e: + except RefreshError as exc: raise GDriveAccessTokenRefreshError( - "Google Drive's access token refreshment is failed", e - ) - except KeyError as e: + "Google Drive's access token refreshment is failed" + ) from exc + except KeyError as exc: raise GDriveMissedCredentialKeyError( "Google Drive's user credentials file '{}' " "misses value for key '{}'".format( - self.gdrive_user_credentials_path, str(e) - ), - e, + self.gdrive_user_credentials_path, str(exc) + ) ) # Handle pydrive.auth.AuthenticationError and others auth failures - except Exception as e: - raise DvcException("Google Drive authentication failed", e) + except Exception as exc: + raise DvcException( + "Google Drive authentication failed" + ) from exc finally: if os.getenv(RemoteGDrive.GDRIVE_USER_CREDENTIALS_DATA): os.remove(self.gdrive_user_credentials_path) diff --git a/dvc/remote/s3.py b/dvc/remote/s3.py index eb7d2e6b2..d4a557532 100644 --- a/dvc/remote/s3.py +++ b/dvc/remote/s3.py @@ -87,8 +87,8 @@ class RemoteS3(RemoteBASE): obj = s3.head_object(Bucket=bucket, Key=path, *args, **kwargs) except Exception as exc: raise DvcException( - "s3://{}/{} does not exist".format(bucket, path), exc - ) + "s3://{}/{} does not exist".format(bucket, path) + ) from exc return obj @classmethod diff --git a/dvc/remote/ssh/connection.py b/dvc/remote/ssh/connection.py index d8461356f..038f74c17 100644 --- a/dvc/remote/ssh/connection.py +++ b/dvc/remote/ssh/connection.py @@ -103,14 +103,13 @@ class SSHConnection: if tail: try: self.sftp.mkdir(path) - except IOError as e: + except IOError as exc: # Since paramiko errors are very vague we need to recheck # whether it's because path already exists or something else - if e.errno == errno.EACCES or not self.exists(path): + if exc.errno == errno.EACCES or not self.exists(path): raise DvcException( - "unable to create remote directory '{}'".format(path), - cause=e, - ) + "unable to create remote directory '{}'".format(path) + ) from exc def walk(self, directory, topdown=True): # NOTE: original os.walk() implementation [1] with default options was @@ -123,9 +122,8 @@ class SSHConnection: raise DvcException( "couldn't get the '{}' remote directory files list".format( directory - ), - cause=exc, - ) + ) + ) from exc dirs = [] nondirs = [] diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 0535df033..3c35683be 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -444,24 +444,24 @@ class Repo(object): cause = None try: out, = self.find_outs_by_path(path) - except OutputNotFoundError as e: + except OutputNotFoundError as exc: out = None - cause = e + cause = exc if out and out.use_cache: try: with self._open_cached(out, remote, mode, encoding) as fd: yield fd return - except FileNotFoundError as e: - raise FileMissingError(relpath(path, self.root_dir), cause=e) + except FileNotFoundError as exc: + raise FileMissingError(relpath(path, self.root_dir)) from exc if self.tree.exists(path): with self.tree.open(path, mode, encoding) as fd: yield fd return - raise FileMissingError(relpath(path, self.root_dir), cause=cause) + raise FileMissingError(relpath(path, self.root_dir)) from cause def _open_cached(self, out, remote=None, mode="r", encoding=None): if out.isdir(): diff --git a/dvc/repo/checkout.py b/dvc/repo/checkout.py index 820184b07..26f9e18e8 100644 --- a/dvc/repo/checkout.py +++ b/dvc/repo/checkout.py @@ -49,7 +49,7 @@ def _checkout( except (StageFileDoesNotExistError, StageFileBadNameError) as exc: if not target: raise - raise CheckoutErrorSuggestGit(target, exc) + raise CheckoutErrorSuggestGit(target) from exc total = get_all_files_numbers(pairs) if total == 0: diff --git a/dvc/repo/reproduce.py b/dvc/repo/reproduce.py index 67e2f3233..c56d14f44 100644 --- a/dvc/repo/reproduce.py +++ b/dvc/repo/reproduce.py @@ -162,6 +162,6 @@ def _reproduce_stages( kwargs["force"] = True result.extend(ret) - except Exception as ex: - raise ReproductionError(st.relpath, ex) + except Exception as exc: + raise ReproductionError(st.relpath) from exc return result diff --git a/dvc/rwlock.py b/dvc/rwlock.py index fdc51b7fd..bad2df949 100644 --- a/dvc/rwlock.py +++ b/dvc/rwlock.py @@ -17,18 +17,17 @@ SCHEMA = Schema({"write": {str: INFO_SCHEMA}, "read": {str: [INFO_SCHEMA]}}) class RWLockFileCorruptedError(DvcException): - def __init__(self, path, cause): + def __init__(self, path): super().__init__( "Unable to read RWLock-file '{}'. JSON structure is " - "corrupted".format(relpath(path)), - cause=cause, + "corrupted".format(relpath(path)) ) class RWLockFileFormatError(DvcException): - def __init__(self, path, cause): + def __init__(self, path): super().__init__( - "RWLock-file '{}' format error.".format(relpath(path)), cause=cause + "RWLock-file '{}' format error.".format(relpath(path)) ) @@ -42,9 +41,9 @@ def _edit_rwlock(lock_dir): except FileNotFoundError: lock = SCHEMA({}) except json.JSONDecodeError as exc: - raise RWLockFileCorruptedError(path, cause=exc) + raise RWLockFileCorruptedError(path) from exc except Invalid as exc: - raise RWLockFileFormatError(path, cause=exc) + raise RWLockFileFormatError(path) from exc lock = defaultdict(dict, lock) lock["read"] = defaultdict(list, lock["read"]) lock["write"] = defaultdict(dict, lock["write"]) diff --git a/dvc/scm/base.py b/dvc/scm/base.py index 482c02e08..412731ca8 100644 --- a/dvc/scm/base.py +++ b/dvc/scm/base.py @@ -22,17 +22,14 @@ class FileNotInCommitError(SCMError): class CloneError(SCMError): - def __init__(self, url, path, cause): - super().__init__( - "Failed to clone repo '{}' to '{}'".format(url, path), cause=cause - ) + def __init__(self, url, path): + super().__init__("Failed to clone repo '{}' to '{}'".format(url, path)) class RevError(SCMError): - def __init__(self, url, rev, cause): + def __init__(self, url, rev): super().__init__( - "Failed to access revision '{}' for repo '{}'".format(rev, url), - cause=cause, + "Failed to access revision '{}' for repo '{}'".format(rev, url) ) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 245c9b7ef..7c432855e 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -85,7 +85,7 @@ class Git(Base): ) tmp_repo.close() except git.exc.GitCommandError as exc: - raise CloneError(url, to_path, exc) + raise CloneError(url, to_path) from exc # NOTE: using our wrapper to make sure that env is fixed in __init__ repo = Git(to_path) @@ -94,7 +94,7 @@ class Git(Base): try: repo.checkout(rev) except git.exc.GitCommandError as exc: - raise RevError(url, rev, exc) + raise RevError(url, rev) from exc return repo @@ -331,8 +331,8 @@ class Git(Base): commits.append(b_commit) trees[DIFF_A_TREE] = self.get_tree(commits[0]) trees[DIFF_B_TREE] = self.get_tree(commits[1]) - except (BadName, BadObject) as e: - raise SCMError("git problem", cause=e) + except (BadName, BadObject) as exc: + raise SCMError("git problem") from exc return trees, commits def get_diff_trees(self, a_ref, b_ref=None): diff --git a/dvc/scm/git/tree.py b/dvc/scm/git/tree.py index c7d2ff26f..6deca3d0d 100644 --- a/dvc/scm/git/tree.py +++ b/dvc/scm/git/tree.py @@ -104,9 +104,8 @@ class GitTree(BaseTree): raise DvcException( "revision '{}' not found in git '{}'".format( self.rev, os.path.relpath(self.git.working_dir) - ), - cause=exc, - ) + ) + ) from exc if not path or path == ".": return tree diff --git a/dvc/system.py b/dvc/system.py index 59b609236..26e8497cd 100644 --- a/dvc/system.py +++ b/dvc/system.py @@ -31,7 +31,7 @@ class System(object): os.link(source, link_name) return except Exception as exc: - raise DvcException("link", cause=exc) + raise DvcException("link") from exc CreateHardLink = ctypes.windll.kernel32.CreateHardLinkW CreateHardLink.argtypes = [ @@ -43,7 +43,7 @@ class System(object): res = CreateHardLink(link_name, source, None) if res == 0: - raise DvcException("CreateHardLinkW", cause=ctypes.WinError()) + raise DvcException("CreateHardLinkW") from ctypes.WinError() @staticmethod def symlink(source, link_name): @@ -69,7 +69,7 @@ class System(object): func.restype = ctypes.c_ubyte if func(link_name, source, flags) == 0: - raise DvcException("CreateSymbolicLinkW", cause=ctypes.WinError()) + raise DvcException("CreateSymbolicLinkW") from ctypes.WinError() @staticmethod def _reflink_darwin(src, dst): diff --git a/dvc/utils/stage.py b/dvc/utils/stage.py index 6913a5771..ce5a79714 100644 --- a/dvc/utils/stage.py +++ b/dvc/utils/stage.py @@ -19,7 +19,7 @@ def parse_stage(text, path): try: return yaml.load(text, Loader=SafeLoader) or {} except yaml.error.YAMLError as exc: - raise StageFileCorruptedError(path, cause=exc) + raise StageFileCorruptedError(path) from exc def parse_stage_for_update(text, path): @@ -35,7 +35,7 @@ def parse_stage_for_update(text, path): yaml = YAML() return yaml.load(text) or {} except YAMLError as exc: - raise StageFileCorruptedError(path, cause=exc) + raise StageFileCorruptedError(path) from exc def dump_stage_file(path, data): diff --git a/tests/func/test_checkout.py b/tests/func/test_checkout.py index 8ca43c24b..db02cdc58 100644 --- a/tests/func/test_checkout.py +++ b/tests/func/test_checkout.py @@ -392,15 +392,15 @@ class TestCheckoutSuggestGit(TestRepro): self.dvc.checkout(targets=["gitbranch"]) except DvcException as exc: self.assertIsInstance(exc, CheckoutErrorSuggestGit) - self.assertIsInstance(exc.cause, StageFileDoesNotExistError) - self.assertIsNone(exc.cause.cause) + self.assertIsInstance(exc.__cause__, StageFileDoesNotExistError) + self.assertIsNone(exc.__cause__.__cause__) try: self.dvc.checkout(targets=[self.FOO]) except DvcException as exc: self.assertIsInstance(exc, CheckoutErrorSuggestGit) - self.assertIsInstance(exc.cause, StageFileBadNameError) - self.assertIsNone(exc.cause.cause) + self.assertIsInstance(exc.__cause__, StageFileBadNameError) + self.assertIsNone(exc.__cause__.__cause__) class TestCheckoutTargetRecursiveShouldNotRemoveOtherUsedFiles(TestDvc): diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index a671b2112..201139f2e 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -107,11 +107,10 @@ class TestColorFormatter: try: raise Exception("first") except Exception as exc: - first_traceback = traceback.format_exc() try: - raise DvcException("second", cause=exc) + raise DvcException("second") from exc except DvcException: - second_traceback = traceback.format_exc() + stack_trace = traceback.format_exc() logger.exception("message") expected = ( @@ -119,13 +118,12 @@ class TestColorFormatter: "{red}{line}{nc}\n" "{stack_trace}" "{red}{line}{nc}\n".format( - line="-" * 60, - stack_trace="\n".join([first_traceback, second_traceback]), - **colors + line="-" * 60, stack_trace=stack_trace, **colors ) ) - assert expected == formatter.format(caplog.records[0]) + assert "Exception: first" in stack_trace + assert "dvc.exceptions.DvcException: second" in stack_trace def test_progress_awareness(self, mocker, capsys, caplog): from dvc.progress import Tqdm