add: rename jobs to remote_jobs to mirror CLI; move validation to CLI (#9550)

- Follow up on https://github.com/iterative/dvc/pull/9522, renames jobs kwarg to remote_jobs
  to mirror CLI.
- Also moves validation logic to CLI.

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:

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:

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")
 

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