diff --git a/dvc/commands/add.py b/dvc/commands/add.py index fd34cbe8b..c21010e04 100644 --- a/dvc/commands/add.py +++ b/dvc/commands/add.py @@ -9,8 +9,41 @@ logger = logging.getLogger(__name__) class CmdAdd(CmdBase): + def validate_args(self) -> None: + from dvc.exceptions import InvalidArgumentError + + args = self.args + invalid_opt = None + + if args.to_remote or args.out: + message = "{option} can't be used with " + message += "--to-remote" if args.to_remote else "--out" + if len(args.targets) != 1: + invalid_opt = "multiple targets" + elif args.glob: + invalid_opt = "--glob option" + elif args.no_commit: + invalid_opt = "--no-commit option" + elif args.external: + invalid_opt = "--external option" + else: + message = "{option} can't be used without --to-remote" + if args.remote: + invalid_opt = "--remote" + elif args.remote_jobs: + invalid_opt = "--remote-jobs" + + if invalid_opt is not None: + raise InvalidArgumentError(message.format(option=invalid_opt)) + def run(self): - from dvc.exceptions import DvcException + from dvc.exceptions import DvcException, InvalidArgumentError + + try: + self.validate_args() + except InvalidArgumentError: + logger.exception("") + return 1 try: self.repo.add( @@ -21,7 +54,7 @@ class CmdAdd(CmdBase): out=self.args.out, remote=self.args.remote, to_remote=self.args.to_remote, - jobs=self.args.remote_jobs, + remote_jobs=self.args.remote_jobs, force=self.args.force, ) except FileNotFoundError: diff --git a/dvc/repo/add.py b/dvc/repo/add.py index 31639b7a3..9ab3d63ea 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -2,7 +2,6 @@ import os from contextlib import contextmanager from typing import ( TYPE_CHECKING, - Any, Dict, Iterator, List, @@ -15,7 +14,6 @@ from typing import ( from dvc.exceptions import ( CacheLinkError, DvcException, - InvalidArgumentError, OutputDuplicationError, OutputNotFoundError, OverlappingOutputPathsError, @@ -47,30 +45,6 @@ def find_targets( return glob_targets(targets_list, glob=glob) -def validate_args(targets: List[str], **kwargs: Any) -> None: - invalid_opt = None - to_remote = kwargs.get("to_remote") - - if to_remote or kwargs.get("out"): - message = "{option} can't be used with " - message += "--to-remote" if to_remote else "-o" - if len(targets) != 1: - invalid_opt = "multiple targets" - elif kwargs.get("no_commit"): - invalid_opt = "--no-commit option" - elif kwargs.get("external"): - invalid_opt = "--external option" - else: - message = "{option} can't be used without --to-remote" - if kwargs.get("remote"): - invalid_opt = "--remote" - elif kwargs.get("jobs"): - invalid_opt = "--remote-jobs" - - if invalid_opt is not None: - raise InvalidArgumentError(message.format(option=invalid_opt)) - - PIPELINE_TRACKED_UPDATE_FMT = ( "cannot update {out!r}: overlaps with an output of {stage} in '{path}'.\n" "Run the pipeline or use 'dvc commit' to force update it." @@ -230,22 +204,13 @@ def add( out: Optional[str] = None, remote: Optional[str] = None, to_remote: bool = False, - jobs: Optional[int] = None, + remote_jobs: Optional[int] = None, force: bool = False, ) -> List["Stage"]: add_targets = find_targets(targets, glob=glob) if not add_targets: return [] - validate_args( - add_targets, - no_commit=no_commit, - external=external, - out=out, - remote=remote, - to_remote=to_remote, - jobs=jobs, - ) stages_with_targets = { target: get_or_create_stage( repo, @@ -264,9 +229,9 @@ def add( repo.check_graph(stages=stages, callback=lambda: st.update("Checking graph")) if to_remote or out: - assert len(stages_with_targets) == 1 + assert len(stages_with_targets) == 1, "multiple targets are unsupported" (source, (stage, _)) = next(iter(stages_with_targets.items())) - _add_transfer(stage, source, remote, to_remote, jobs=jobs, force=force) + _add_transfer(stage, source, remote, to_remote, jobs=remote_jobs, force=force) return [stage] with warn_link_failures() as link_failures: diff --git a/tests/func/test_add.py b/tests/func/test_add.py index faa5ad743..84cc0bf23 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -13,11 +13,7 @@ from dvc.cachemgr import CacheManager from dvc.cli import main from dvc.config import ConfigError from dvc.dvcfile import DVC_FILE_SUFFIX -from dvc.exceptions import ( - DvcException, - InvalidArgumentError, - OverlappingOutputPathsError, -) +from dvc.exceptions import DvcException, OverlappingOutputPathsError from dvc.fs import LocalFileSystem, system from dvc.output import ( OutputAlreadyTrackedError, @@ -874,19 +870,6 @@ def test_add_to_remote_absolute(tmp_dir, make_tmp_dir, dvc, remote): dvc.add(str(tmp_foo), out=str(tmp_bar), to_remote=True) -@pytest.mark.parametrize( - "invalid_opt, kwargs", - [ - ("multiple targets", {"targets": ["foo", "bar", "baz"]}), - ("--no-commit", {"targets": ["foo"], "no_commit": True}), - ("--external", {"targets": ["foo"], "external": True}), - ], -) -def test_add_to_remote_invalid_combinations(dvc, invalid_opt, kwargs): - with pytest.raises(InvalidArgumentError, match=invalid_opt): - dvc.add(to_remote=True, **kwargs) - - def test_add_to_cache_dir(tmp_dir, dvc, local_cloud): local_cloud.gen({"data": {"foo": "foo", "bar": "bar"}}) @@ -962,18 +945,6 @@ def test_add_to_cache_not_exists(tmp_dir, dvc, local_cloud): assert dest_dir.with_suffix(".dvc").exists() -@pytest.mark.parametrize( - "invalid_opt, kwargs", - [ - ("multiple targets", {"targets": ["foo", "bar", "baz"]}), - ("--no-commit", {"targets": ["foo"], "no_commit": True}), - ], -) -def test_add_to_cache_invalid_combinations(dvc, invalid_opt, kwargs): - with pytest.raises(InvalidArgumentError, match=invalid_opt): - dvc.add(out="bar", **kwargs) - - def test_add_to_cache_from_remote(tmp_dir, dvc, workspace): workspace.gen("foo", "foo") diff --git a/tests/unit/command/test_add.py b/tests/unit/command/test_add.py index 60ef72652..398eb3b47 100644 --- a/tests/unit/command/test_add.py +++ b/tests/unit/command/test_add.py @@ -29,7 +29,7 @@ def test_add(mocker, dvc): out=None, remote=None, to_remote=False, - jobs=None, + remote_jobs=None, force=False, ) @@ -61,7 +61,7 @@ def test_add_to_remote(mocker): out="bar", remote="remote", to_remote=True, - jobs=None, + remote_jobs=None, force=False, ) @@ -93,5 +93,5 @@ def test_add_to_cache_invalid_combinations(mocker, caplog): cmd = cli_args.func(cli_args) with caplog.at_level(logging.ERROR, logger="dvc"): assert cmd.run() == 1 - expected_msg = "multiple targets can't be used with -o" + expected_msg = "multiple targets can't be used with --out" assert expected_msg in caplog.text