diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index 0b8b9551d..5c274cfee 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -28,6 +28,7 @@ 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") @@ -66,7 +67,7 @@ class CmdDataFetch(CmdDataBase): all_tags=self.args.all_tags, with_deps=self.args.with_deps, recursive=self.args.recursive, - trust_remote=self.args.trust_remote, + verify=self.args.verify, ) except DvcException: logger.exception("failed to fetch data from the cloud") @@ -146,6 +147,12 @@ 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 @@ -233,10 +240,10 @@ def add_parser(subparsers, _parent_parser): help="Fetch cache for subdirectories of specified directory.", ) fetch_parser.add_argument( - "--trust-remote", + "--verify", action="store_true", default=False, - help="Trust remote cache checksums upon fetching.", + help="Verify downloaded files checksums.", ) fetch_parser.set_defaults(func=CmdDataFetch) diff --git a/dvc/data_cloud.py b/dvc/data_cloud.py index d83275327..fc4a53f33 100644 --- a/dvc/data_cloud.py +++ b/dvc/data_cloud.py @@ -63,12 +63,7 @@ class DataCloud(object): ) def pull( - self, - cache, - jobs=None, - remote=None, - show_checksums=False, - trust_remote=False, + self, cache, jobs=None, remote=None, show_checksums=False, verify=False ): """Pull data items in a cloud-agnostic way. @@ -86,15 +81,18 @@ class DataCloud(object): remote=self.get_remote(remote, "pull"), show_checksums=show_checksums, ) - if trust_remote: - for checksum in cache["local"].keys(): - cache_file = self.repo.cache.local.checksum_to_path_info( - checksum - ) - if self.repo.cache.local.exists(cache_file): - self.repo.state.save(cache_file, checksum) + + if not verify: + self._save_pulled_checksums(cache) + return downloaded_items_num + def _save_pulled_checksums(self, cache): + for checksum in cache["local"].keys(): + cache_file = self.repo.cache.local.checksum_to_path_info(checksum) + if self.repo.cache.local.exists(cache_file): + self.repo.state.save(cache_file, checksum) + def status(self, cache, jobs=None, remote=None, show_checksums=False): """Check status of data items in a cloud-agnostic way. diff --git a/dvc/output/base.py b/dvc/output/base.py index beb175b80..04d9ed461 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -345,7 +345,7 @@ class OutputBase(object): force=False, jobs=None, filter_info=None, - trust_remote=False, + verify=False, ): """Get a list of `info`s related to the given directory. @@ -375,7 +375,7 @@ class OutputBase(object): jobs=jobs, remote=remote, show_checksums=False, - trust_remote=trust_remote, + verify=verify, ) except DvcException: logger.debug("failed to pull cache for '{}'".format(self)) diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 895928cd6..96663f67f 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -223,7 +223,7 @@ class Repo(object): force=False, jobs=None, recursive=False, - trust_remote=False, + verify=False, ): """Get the stages related to the given target and collect the `info` of its outputs. @@ -263,7 +263,7 @@ class Repo(object): force=force, jobs=jobs, filter_info=filter_info, - trust_remote=trust_remote, + verify=verify, ) cache.update(used_cache, suffix=suffix) diff --git a/dvc/repo/fetch.py b/dvc/repo/fetch.py index 1064b4915..37fb0a56b 100644 --- a/dvc/repo/fetch.py +++ b/dvc/repo/fetch.py @@ -20,7 +20,7 @@ def _fetch( with_deps=False, all_tags=False, recursive=False, - trust_remote=False, + verify=False, ): """Download data items from a cloud and imported repositories @@ -43,7 +43,7 @@ def _fetch( remote=remote, jobs=jobs, recursive=recursive, - trust_remote=trust_remote, + verify=verify, ) downloaded = 0 @@ -55,7 +55,7 @@ def _fetch( jobs, remote=remote, show_checksums=show_checksums, - trust_remote=trust_remote, + verify=verify, ) except NoRemoteError: if not used.external and used["local"]: diff --git a/dvc/repo/pull.py b/dvc/repo/pull.py index 474cecda7..52779376d 100644 --- a/dvc/repo/pull.py +++ b/dvc/repo/pull.py @@ -17,7 +17,7 @@ def pull( all_tags=False, force=False, recursive=False, - trust_remote=False, + verify=False, ): processed_files_count = self._fetch( targets, @@ -27,7 +27,7 @@ def pull( all_tags=all_tags, with_deps=with_deps, recursive=recursive, - trust_remote=trust_remote, + 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 fd9348cd9..8f9d12b9f 100644 --- a/tests/func/test_remote.py +++ b/tests/func/test_remote.py @@ -260,10 +260,12 @@ def test_modify_missing_remote(dvc): remote_config.modify("myremote", "gdrive_client_id", "xxx") -def test_trust_remote_checksums(tmp_dir, tmp_path_factory, erepo_dir, mocker): +def test_verify_checksums(tmp_dir, tmp_path_factory, erepo_dir, mocker): with erepo_dir.chdir(): - - erepo_dir.dvc_gen({"file": "file content"}, commit="add dir") + erepo_dir.dvc_gen({"file": "file1 content"}, commit="add file") + erepo_dir.dvc_gen( + {"dir": {"subfile": "file2 content"}}, commit="add " "dir" + ) Git.clone(fspath(erepo_dir), fspath(tmp_dir)) @@ -271,49 +273,14 @@ def test_trust_remote_checksums(tmp_dir, tmp_path_factory, erepo_dir, mocker): dvc = Repo(fspath(tmp_dir)) - md5_spy = mocker.spy(dvc.cache.local, "get_file_checksum") + file_md5_spy = mocker.spy(dvc.cache.local, "get_file_checksum") - dvc.pull(trust_remote=True) + dvc.pull() + assert file_md5_spy.call_count == 0 - assert md5_spy.call_count == 0 + # Removing cache will invalidate existing state entries + shutil.rmtree(dvc.cache.local.cache_dir) - -def test_trust_remote_checksums_dir( - tmp_dir, mocker, tmp_path_factory, erepo_dir -): - with erepo_dir.chdir(): - erepo_dir.dvc_gen({"dir": {"file": "file content"}}, commit="add dir") - - Git.clone(fspath(erepo_dir), fspath(tmp_dir)) - - from dvc.repo import Repo - - dvc = Repo(fspath(tmp_dir)) - - md5_spy = mocker.spy(dvc.cache.local, "get_file_checksum") - - dvc.pull(trust_remote=False) - - assert md5_spy.call_count == 0 - - -def test_trust_remote_checksums_external_dep( - tmp_dir, mocker, tmp_path_factory, erepo_dir -): - external_path = tmp_path_factory.mktemp("external_dep") / "file" - external_path.write_text("file content") - - with erepo_dir.chdir(): - erepo_dir.dvc_add(fspath(external_path), commit="add external file") - - Git.clone(fspath(erepo_dir), fspath(tmp_dir)) - - from dvc.repo import Repo - - dvc = Repo(fspath(tmp_dir)) - - md5_spy = mocker.spy(dvc.cache.local, "get_file_checksum") - - dvc.pull(trust_remote=True) - - assert md5_spy.call_count == 0 + dvc.pull(verify=True) + assert file_md5_spy.call_count == 3 + pass