From a148d16a9550cf22abdcb4e6093452c0bf7acfa0 Mon Sep 17 00:00:00 2001 From: pawel Date: Tue, 21 Jan 2020 13:35:25 +0100 Subject: [PATCH] data cloud: verify is config option now --- dvc/command/data_sync.py | 14 ------ dvc/config.py | 2 + dvc/data_cloud.py | 12 ++--- dvc/output/base.py | 8 +-- dvc/remote/base.py | 4 ++ dvc/remote/gdrive.py | 1 + dvc/repo/__init__.py | 2 - dvc/repo/fetch.py | 8 +-- dvc/repo/pull.py | 2 - tests/func/test_remote.py | 7 ++- tests/unit/command/test_data_sync.py | 74 ---------------------------- 11 files changed, 19 insertions(+), 115 deletions(-) delete mode 100644 tests/unit/command/test_data_sync.py diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index 5c274cfee..9442e2bbb 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -28,7 +28,6 @@ class CmdDataPull(CmdDataBase): with_deps=self.args.with_deps, force=self.args.force, recursive=self.args.recursive, - verify=self.args.verify, ) except DvcException: logger.exception("failed to pull data from the cloud") @@ -67,7 +66,6 @@ class CmdDataFetch(CmdDataBase): all_tags=self.args.all_tags, with_deps=self.args.with_deps, recursive=self.args.recursive, - verify=self.args.verify, ) except DvcException: logger.exception("failed to fetch data from the cloud") @@ -147,12 +145,6 @@ def add_parser(subparsers, _parent_parser): default=False, help="Pull cache for subdirectories of the specified directory.", ) - pull_parser.add_argument( - "--verify", - action="store_true", - default=False, - help="Verify downloaded files checksums.", - ) pull_parser.set_defaults(func=CmdDataPull) # Push @@ -239,12 +231,6 @@ def add_parser(subparsers, _parent_parser): default=False, help="Fetch cache for subdirectories of specified directory.", ) - fetch_parser.add_argument( - "--verify", - action="store_true", - default=False, - help="Verify downloaded files checksums.", - ) fetch_parser.set_defaults(func=CmdDataFetch) # Status diff --git a/dvc/config.py b/dvc/config.py index 65250c76d..0759395d0 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -198,6 +198,7 @@ class Config(object): # pylint: disable=too-many-instance-attributes SECTION_REMOTE_ASK_PASSWORD = "ask_password" SECTION_REMOTE_GSS_AUTH = "gss_auth" SECTION_REMOTE_NO_TRAVERSE = "no_traverse" + SECTION_REMOTE_VERIFY = "verify" SECTION_REMOTE_SCHEMA = { Required(SECTION_REMOTE_URL): str, SECTION_AWS_REGION: str, @@ -232,6 +233,7 @@ class Config(object): # pylint: disable=too-many-instance-attributes SECTION_GDRIVE_USER_CREDENTIALS_FILE: str, PRIVATE_CWD: str, SECTION_REMOTE_NO_TRAVERSE: Bool, + SECTION_REMOTE_VERIFY: Bool, } SECTION_STATE = "state" diff --git a/dvc/data_cloud.py b/dvc/data_cloud.py index fc4a53f33..a4d67db35 100644 --- a/dvc/data_cloud.py +++ b/dvc/data_cloud.py @@ -62,9 +62,7 @@ class DataCloud(object): show_checksums=show_checksums, ) - def pull( - self, cache, jobs=None, remote=None, show_checksums=False, verify=False - ): + def pull(self, cache, jobs=None, remote=None, show_checksums=False): """Pull data items in a cloud-agnostic way. Args: @@ -75,14 +73,12 @@ class DataCloud(object): show_checksums (bool): show checksums instead of file names in information messages. """ + remote = self.get_remote(remote, "pull") downloaded_items_num = self.repo.cache.local.pull( - cache, - jobs=jobs, - remote=self.get_remote(remote, "pull"), - show_checksums=show_checksums, + cache, jobs=jobs, remote=remote, show_checksums=show_checksums ) - if not verify: + if not remote.verify: self._save_pulled_checksums(cache) return downloaded_items_num diff --git a/dvc/output/base.py b/dvc/output/base.py index 04d9ed461..0fca876c4 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -340,12 +340,7 @@ class OutputBase(object): self.remote.unprotect(self.path_info) def _collect_used_dir_cache( - self, - remote=None, - force=False, - jobs=None, - filter_info=None, - verify=False, + self, remote=None, force=False, jobs=None, filter_info=None ): """Get a list of `info`s related to the given directory. @@ -375,7 +370,6 @@ class OutputBase(object): jobs=jobs, remote=remote, show_checksums=False, - verify=verify, ) except DvcException: logger.debug("failed to pull cache for '{}'".format(self)) diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 788742c8c..7262ba1a9 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -80,6 +80,7 @@ class RemoteBASE(object): CHECKSUM_JOBS = max(1, min(4, cpu_count() // 2)) DEFAULT_CACHE_TYPES = ["copy"] DEFAULT_NO_TRAVERSE = True + DEFAULT_VERIFY = False state = StateNoop() @@ -93,6 +94,9 @@ class RemoteBASE(object): self.no_traverse = config.get( Config.SECTION_REMOTE_NO_TRAVERSE, self.DEFAULT_NO_TRAVERSE ) + self.verify = config.get( + Config.SECTION_REMOTE_VERIFY, self.DEFAULT_VERIFY + ) self._dir_info = {} types = config.get(Config.SECTION_CACHE_TYPE, None) diff --git a/dvc/remote/gdrive.py b/dvc/remote/gdrive.py index a38b2a1a2..24d65aa25 100644 --- a/dvc/remote/gdrive.py +++ b/dvc/remote/gdrive.py @@ -63,6 +63,7 @@ class RemoteGDrive(RemoteBASE): path_cls = CloudURLInfo REQUIRES = {"pydrive2": "pydrive2"} DEFAULT_NO_TRAVERSE = False + DEFAULT_VERIFY = True GDRIVE_USER_CREDENTIALS_DATA = "GDRIVE_USER_CREDENTIALS_DATA" DEFAULT_USER_CREDENTIALS_FILE = "gdrive-user-credentials.json" diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 96663f67f..9d0a506a8 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -223,7 +223,6 @@ class Repo(object): force=False, jobs=None, recursive=False, - verify=False, ): """Get the stages related to the given target and collect the `info` of its outputs. @@ -263,7 +262,6 @@ class Repo(object): force=force, jobs=jobs, filter_info=filter_info, - verify=verify, ) cache.update(used_cache, suffix=suffix) diff --git a/dvc/repo/fetch.py b/dvc/repo/fetch.py index 37fb0a56b..e995630e4 100644 --- a/dvc/repo/fetch.py +++ b/dvc/repo/fetch.py @@ -20,7 +20,6 @@ def _fetch( with_deps=False, all_tags=False, recursive=False, - verify=False, ): """Download data items from a cloud and imported repositories @@ -43,7 +42,6 @@ def _fetch( remote=remote, jobs=jobs, recursive=recursive, - verify=verify, ) downloaded = 0 @@ -51,11 +49,7 @@ def _fetch( try: downloaded += self.cloud.pull( - used, - jobs, - remote=remote, - show_checksums=show_checksums, - verify=verify, + used, jobs, remote=remote, show_checksums=show_checksums ) except NoRemoteError: if not used.external and used["local"]: diff --git a/dvc/repo/pull.py b/dvc/repo/pull.py index 52779376d..fadd34d3d 100644 --- a/dvc/repo/pull.py +++ b/dvc/repo/pull.py @@ -17,7 +17,6 @@ def pull( all_tags=False, force=False, recursive=False, - verify=False, ): processed_files_count = self._fetch( targets, @@ -27,7 +26,6 @@ def pull( all_tags=all_tags, with_deps=with_deps, recursive=recursive, - verify=verify, ) self._checkout( targets=targets, with_deps=with_deps, force=force, recursive=recursive diff --git a/tests/func/test_remote.py b/tests/func/test_remote.py index 7639d6b88..44580e571 100644 --- a/tests/func/test_remote.py +++ b/tests/func/test_remote.py @@ -280,6 +280,11 @@ def test_verify_checksums(tmp_dir, erepo_dir, mocker): # Removing cache will invalidate existing state entries shutil.rmtree(dvc.cache.local.cache_dir) + dvc.config.set( + Config.SECTION_REMOTE_FMT.format("upstream"), + Config.SECTION_REMOTE_VERIFY, + "True", + ) - dvc.pull(verify=True) + dvc.pull() assert file_md5_spy.call_count == 3 diff --git a/tests/unit/command/test_data_sync.py b/tests/unit/command/test_data_sync.py deleted file mode 100644 index 8565c2e83..000000000 --- a/tests/unit/command/test_data_sync.py +++ /dev/null @@ -1,74 +0,0 @@ -from dvc.cli import parse_args -from dvc.command.data_sync import CmdDataFetch, CmdDataPull - - -def test_fetch(mocker): - cli_args = parse_args( - [ - "fetch", - "target.dvc", - "-r", - "remote", - "-a", - "-T", - "-d", - "-R", - "--verify", - "-j", - "2", - ] - ) - assert cli_args.func == CmdDataFetch - - cmd = cli_args.func(cli_args) - m = mocker.patch("dvc.repo.Repo.fetch") - - assert cmd.run() == 0 - - m.assert_called_once_with( - targets=["target.dvc"], - remote="remote", - all_branches=True, - all_tags=True, - with_deps=True, - recursive=True, - verify=True, - jobs=2, - ) - - -def test_pull(mocker): - cli_args = parse_args( - [ - "pull", - "target.dvc", - "-r", - "remote", - "-a", - "-T", - "-d", - "-R", - "-f", - "--verify", - "-j", - "2", - ] - ) - assert cli_args.func == CmdDataPull - - cmd = cli_args.func(cli_args) - m = mocker.patch("dvc.repo.Repo.pull") - - assert cmd.run() == 0 - - m.assert_called_once_with( - targets=["target.dvc"], - remote="remote", - all_branches=True, - all_tags=True, - with_deps=True, - recursive=True, - force=True, - verify=True, - jobs=2, - )