From 8a869dd6619526091c21f18dd1640b8cc9af0e0c Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Tue, 3 May 2022 13:59:15 +0530 Subject: [PATCH 1/6] Fail on sync when there is no match for glob. --- dvc/exceptions.py | 6 ++++++ dvc/repo/pull.py | 4 ++++ dvc/repo/push.py | 9 ++++++++- tests/func/test_import.py | 21 ++++++++++++++++++++- 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 127d46362d..26afde5858 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -349,3 +349,9 @@ def __init__(self, dep, a, b): class PrettyDvcException(DvcException): def __pretty_exc__(self, **kwargs): """Print prettier exception message.""" + + +class GlobDoesNotMatchError(DvcException): + def __init__(self, glob): + msg = f"Glob {glob} has no matches." + super().__init__(msg) diff --git a/dvc/repo/pull.py b/dvc/repo/pull.py index b4f472933f..3639a89b61 100644 --- a/dvc/repo/pull.py +++ b/dvc/repo/pull.py @@ -1,6 +1,7 @@ import logging from typing import TYPE_CHECKING, Optional +from dvc.exceptions import GlobDoesNotMatchError from dvc.repo import locked from dvc.utils import glob_targets @@ -31,6 +32,9 @@ def pull( expanded_targets = glob_targets(targets, glob=glob) + if targets and not expanded_targets: + raise GlobDoesNotMatchError(targets) + processed_files_count = self.fetch( expanded_targets, jobs, diff --git a/dvc/repo/push.py b/dvc/repo/push.py index 25a9b36922..2d9a7fbd18 100644 --- a/dvc/repo/push.py +++ b/dvc/repo/push.py @@ -1,6 +1,10 @@ from typing import TYPE_CHECKING, Optional -from dvc.exceptions import FileTransferError, UploadError +from dvc.exceptions import ( + FileTransferError, + GlobDoesNotMatchError, + UploadError, +) from ..utils import glob_targets from . import locked @@ -35,6 +39,9 @@ def push( expanded_targets = glob_targets(targets, glob=glob) + if targets and not expanded_targets: + raise GlobDoesNotMatchError(targets) + used = self.used_objs( expanded_targets, all_branches=all_branches, diff --git a/tests/func/test_import.py b/tests/func/test_import.py index 1c9dd986ed..afa46592e0 100644 --- a/tests/func/test_import.py +++ b/tests/func/test_import.py @@ -9,7 +9,11 @@ from dvc.config import NoRemoteError from dvc.data.db import ODBManager from dvc.dvcfile import Dvcfile -from dvc.exceptions import DownloadError, PathMissingError +from dvc.exceptions import ( + DownloadError, + GlobDoesNotMatchError, + PathMissingError, +) from dvc.stage.exceptions import StagePathNotFoundError from dvc.system import System from dvc.utils.fs import makedirs, remove @@ -266,6 +270,21 @@ def test_pull_wildcard_imported_directory_stage(tmp_dir, dvc, erepo_dir): assert (tmp_dir / "dir_imported123").read_text() == {"foo": "foo content"} +def test_push_pull_glob_no_match( + tmp_dir, dvc, erepo_dir, run_copy, local_remote +): + tmp_dir.gen("foo", "foo") + erepo_dir.add_remote(config=local_remote.config) + + with erepo_dir.chdir(): + with pytest.raises(GlobDoesNotMatchError): + dvc.push(targets="b*", glob=True) + + with pytest.raises(GlobDoesNotMatchError): + dvc.pull(targets="b*", glob=True) + assert not os.path.exists(erepo_dir.dvc.stage_cache.cache_dir) + + def test_push_wildcard_from_bare_git_repo( tmp_dir, make_tmp_dir, erepo_dir, local_cloud ): From b67f5501c73fa7a0ed0c72869fceca776b97d723 Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Fri, 6 May 2022 23:57:22 +0530 Subject: [PATCH 2/6] Move exception raising to glob_targets to cover all commands. --- dvc/repo/pull.py | 4 ---- dvc/repo/push.py | 9 +-------- dvc/utils/__init__.py | 9 ++++++++- tests/func/test_add.py | 6 ++++++ 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/dvc/repo/pull.py b/dvc/repo/pull.py index 3639a89b61..b4f472933f 100644 --- a/dvc/repo/pull.py +++ b/dvc/repo/pull.py @@ -1,7 +1,6 @@ import logging from typing import TYPE_CHECKING, Optional -from dvc.exceptions import GlobDoesNotMatchError from dvc.repo import locked from dvc.utils import glob_targets @@ -32,9 +31,6 @@ def pull( expanded_targets = glob_targets(targets, glob=glob) - if targets and not expanded_targets: - raise GlobDoesNotMatchError(targets) - processed_files_count = self.fetch( expanded_targets, jobs, diff --git a/dvc/repo/push.py b/dvc/repo/push.py index 2d9a7fbd18..25a9b36922 100644 --- a/dvc/repo/push.py +++ b/dvc/repo/push.py @@ -1,10 +1,6 @@ from typing import TYPE_CHECKING, Optional -from dvc.exceptions import ( - FileTransferError, - GlobDoesNotMatchError, - UploadError, -) +from dvc.exceptions import FileTransferError, UploadError from ..utils import glob_targets from . import locked @@ -39,9 +35,6 @@ def push( expanded_targets = glob_targets(targets, glob=glob) - if targets and not expanded_targets: - raise GlobDoesNotMatchError(targets) - used = self.used_objs( expanded_targets, all_branches=all_branches, diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 81157ed997..923a57e534 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -476,17 +476,24 @@ def is_exec(mode): def glob_targets(targets, glob=True, recursive=True): + from dvc.exceptions import GlobDoesNotMatchError + if not glob: return targets from glob import iglob - return [ + results = [ exp_target for target in targets for exp_target in iglob(target, recursive=recursive) ] + if not results: + raise GlobDoesNotMatchError(targets) + + return results + def error_handler(func): def wrapper(*args, **kwargs): diff --git a/tests/func/test_add.py b/tests/func/test_add.py index 610710b482..684991517b 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -16,6 +16,7 @@ from dvc.dvcfile import DVC_FILE_SUFFIX from dvc.exceptions import ( DvcException, + GlobDoesNotMatchError, InvalidArgumentError, OutputDuplicationError, OverlappingOutputPathsError, @@ -292,6 +293,11 @@ def test_add_filtered_files_in_dir( assert stage.outs[0].def_path in expected_def_paths +def test_add_glob_no_result_exception(dvc): + with pytest.raises(GlobDoesNotMatchError): + dvc.add(["invalid*"], glob=True) + + class TestAddExternal(TestAdd): @pytest.fixture def hash_name(self): From 394b82ae8c69dcb20b980e2eaf343e42bcad15cc Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Sat, 7 May 2022 00:39:09 +0530 Subject: [PATCH 3/6] Move test to test_data_cloud. --- tests/func/test_data_cloud.py | 16 ++++++++++++++++ tests/func/test_import.py | 21 +-------------------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index eea0e549b4..629db91f11 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -8,6 +8,7 @@ import dvc as dvc_module from dvc.cli import main from dvc.data.db.local import LocalObjectDB +from dvc.exceptions import GlobDoesNotMatchError from dvc.external_repo import clean_repos from dvc.objects.db import ObjectDB from dvc.stage.exceptions import StageNotFound @@ -570,3 +571,18 @@ def test_target_remote(tmp_dir, dvc, make_remote): "6b18131dc289fd37006705affe961ef8.dir", "b8a9f715dbb64fd5c56e7783c6820a61", } + + +def test_push_pull_glob_no_match( + tmp_dir, dvc, erepo_dir, run_copy, local_remote +): + tmp_dir.gen("foo", "foo") + erepo_dir.add_remote(config=local_remote.config) + + with erepo_dir.chdir(): + with pytest.raises(GlobDoesNotMatchError): + dvc.push(targets="b*", glob=True) + + with pytest.raises(GlobDoesNotMatchError): + dvc.pull(targets="b*", glob=True) + assert not os.path.exists(erepo_dir.dvc.stage_cache.cache_dir) diff --git a/tests/func/test_import.py b/tests/func/test_import.py index afa46592e0..1c9dd986ed 100644 --- a/tests/func/test_import.py +++ b/tests/func/test_import.py @@ -9,11 +9,7 @@ from dvc.config import NoRemoteError from dvc.data.db import ODBManager from dvc.dvcfile import Dvcfile -from dvc.exceptions import ( - DownloadError, - GlobDoesNotMatchError, - PathMissingError, -) +from dvc.exceptions import DownloadError, PathMissingError from dvc.stage.exceptions import StagePathNotFoundError from dvc.system import System from dvc.utils.fs import makedirs, remove @@ -270,21 +266,6 @@ def test_pull_wildcard_imported_directory_stage(tmp_dir, dvc, erepo_dir): assert (tmp_dir / "dir_imported123").read_text() == {"foo": "foo content"} -def test_push_pull_glob_no_match( - tmp_dir, dvc, erepo_dir, run_copy, local_remote -): - tmp_dir.gen("foo", "foo") - erepo_dir.add_remote(config=local_remote.config) - - with erepo_dir.chdir(): - with pytest.raises(GlobDoesNotMatchError): - dvc.push(targets="b*", glob=True) - - with pytest.raises(GlobDoesNotMatchError): - dvc.pull(targets="b*", glob=True) - assert not os.path.exists(erepo_dir.dvc.stage_cache.cache_dir) - - def test_push_wildcard_from_bare_git_repo( tmp_dir, make_tmp_dir, erepo_dir, local_cloud ): From 9f5cb58693e4afe1527143c13f00db52fb67df02 Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Sat, 7 May 2022 00:45:09 +0530 Subject: [PATCH 4/6] Update tests/func/test_data_cloud.py Co-authored-by: David de la Iglesia Castro --- tests/func/test_data_cloud.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index 629db91f11..dd1a4f905a 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -574,15 +574,10 @@ def test_target_remote(tmp_dir, dvc, make_remote): def test_push_pull_glob_no_match( - tmp_dir, dvc, erepo_dir, run_copy, local_remote + tmp_dir, dvc, ): - tmp_dir.gen("foo", "foo") - erepo_dir.add_remote(config=local_remote.config) + with pytest.raises(GlobDoesNotMatchError): + dvc.push(targets="b*", glob=True) - with erepo_dir.chdir(): - with pytest.raises(GlobDoesNotMatchError): - dvc.push(targets="b*", glob=True) - - with pytest.raises(GlobDoesNotMatchError): - dvc.pull(targets="b*", glob=True) - assert not os.path.exists(erepo_dir.dvc.stage_cache.cache_dir) + with pytest.raises(GlobDoesNotMatchError): + dvc.pull(targets="b*", glob=True) From 68a1883b4a46deb151af7bddc86d70f296fc6662 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 6 May 2022 19:15:55 +0000 Subject: [PATCH 5/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/func/test_data_cloud.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index dd1a4f905a..42657cf60f 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -574,7 +574,8 @@ def test_target_remote(tmp_dir, dvc, make_remote): def test_push_pull_glob_no_match( - tmp_dir, dvc, + tmp_dir, + dvc, ): with pytest.raises(GlobDoesNotMatchError): dvc.push(targets="b*", glob=True) From b7c9a9f86e59419c57b19cc8524e9fdddd6ded41 Mon Sep 17 00:00:00 2001 From: Karthikeyan Singaravelan Date: Tue, 10 May 2022 11:16:16 +0530 Subject: [PATCH 6/6] Revert other tests and test only glob exception. --- dvc/exceptions.py | 6 ------ dvc/utils/__init__.py | 5 +++-- tests/func/test_add.py | 6 ------ tests/func/test_data_cloud.py | 12 ------------ tests/func/test_utils.py | 12 ++++++++++++ 5 files changed, 15 insertions(+), 26 deletions(-) diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 26afde5858..127d46362d 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -349,9 +349,3 @@ def __init__(self, dep, a, b): class PrettyDvcException(DvcException): def __pretty_exc__(self, **kwargs): """Print prettier exception message.""" - - -class GlobDoesNotMatchError(DvcException): - def __init__(self, glob): - msg = f"Glob {glob} has no matches." - super().__init__(msg) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 923a57e534..05e3545f6b 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -476,7 +476,7 @@ def is_exec(mode): def glob_targets(targets, glob=True, recursive=True): - from dvc.exceptions import GlobDoesNotMatchError + from ..exceptions import DvcException if not glob: return targets @@ -490,7 +490,8 @@ def glob_targets(targets, glob=True, recursive=True): ] if not results: - raise GlobDoesNotMatchError(targets) + msg = f"Glob {targets} has no matches." + raise DvcException(msg) return results diff --git a/tests/func/test_add.py b/tests/func/test_add.py index 684991517b..610710b482 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -16,7 +16,6 @@ from dvc.dvcfile import DVC_FILE_SUFFIX from dvc.exceptions import ( DvcException, - GlobDoesNotMatchError, InvalidArgumentError, OutputDuplicationError, OverlappingOutputPathsError, @@ -293,11 +292,6 @@ def test_add_filtered_files_in_dir( assert stage.outs[0].def_path in expected_def_paths -def test_add_glob_no_result_exception(dvc): - with pytest.raises(GlobDoesNotMatchError): - dvc.add(["invalid*"], glob=True) - - class TestAddExternal(TestAdd): @pytest.fixture def hash_name(self): diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index 42657cf60f..eea0e549b4 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -8,7 +8,6 @@ import dvc as dvc_module from dvc.cli import main from dvc.data.db.local import LocalObjectDB -from dvc.exceptions import GlobDoesNotMatchError from dvc.external_repo import clean_repos from dvc.objects.db import ObjectDB from dvc.stage.exceptions import StageNotFound @@ -571,14 +570,3 @@ def test_target_remote(tmp_dir, dvc, make_remote): "6b18131dc289fd37006705affe961ef8.dir", "b8a9f715dbb64fd5c56e7783c6820a61", } - - -def test_push_pull_glob_no_match( - tmp_dir, - dvc, -): - with pytest.raises(GlobDoesNotMatchError): - dvc.push(targets="b*", glob=True) - - with pytest.raises(GlobDoesNotMatchError): - dvc.pull(targets="b*", glob=True) diff --git a/tests/func/test_utils.py b/tests/func/test_utils.py index 026357afd8..7910f4fbac 100644 --- a/tests/func/test_utils.py +++ b/tests/func/test_utils.py @@ -1,4 +1,9 @@ +import re + +import pytest + from dvc import utils +from dvc.exceptions import DvcException from dvc.fs.local import LocalFileSystem @@ -42,3 +47,10 @@ def test_boxify(): ) assert expected == utils.boxify("message") + + +def test_glob_no_match(): + with pytest.raises( + DvcException, match=re.escape("Glob ['invalid*'] has no matches.") + ): + utils.glob_targets(["invalid*"], glob=True)