ui: merge --to-remote and --remote for add and import-url

dvc/command/add.py

@@ -11,24 +11,38 @@ class CmdAdd(CmdBase):
     def run(self):
         from dvc.exceptions import (
             DvcException,
+            InvalidArgumentError,
             RecursiveAddingWhileUsingFilename,
         )
 
         try:
             if len(self.args.targets) > 1 and self.args.file:
                 raise RecursiveAddingWhileUsingFilename()
 
+            # Workaround to give priority to target over --to-remote.
+            targets = self.args.targets
+            to_remote = self.args.to_remote
+            if len(targets) == 0:
+                # --to-remote has captured the target as its own
+                # argument, so we move it to the target.
+                if isinstance(to_remote, str):
+                    targets = [to_remote]
+                    to_remote = True
+                else:
+                    raise InvalidArgumentError(
+                        "the following arguments are required: targets"
+                    )
+
             self.repo.add(
-                self.args.targets,
+                targets,
                 recursive=self.args.recursive,
                 no_commit=self.args.no_commit,
                 fname=self.args.file,
                 external=self.args.external,
                 glob=self.args.glob,
                 desc=self.args.desc,
                 out=self.args.out,
-                remote=self.args.remote,
-                to_remote=self.args.to_remote,
+                to_remote=to_remote,
                 jobs=self.args.jobs,
             )
 
@@ -86,14 +100,10 @@ def add_parser(subparsers, parent_parser):
     )
     parser.add_argument(
         "--to-remote",
-        action="store_true",
+        nargs="?",
         default=False,
+        const=True,
         help="Download it directly to the remote",
-    )
-    parser.add_argument(
-        "-r",
-        "--remote",
-        help="Remote storage to download to",
         metavar="<name>",
     )
     parser.add_argument(
@@ -117,6 +127,6 @@ def add_parser(subparsers, parent_parser):
         ),
     )
     parser.add_argument(
-        "targets", nargs="+", help="Input files/directories to add.",
+        "targets", nargs="*", help="Input files/directories to add.",
     ).complete = completion.FILE
     parser.set_defaults(func=CmdAdd)

dvc/command/imp_url.py

@@ -16,7 +16,6 @@ def run(self):
                 out=self.args.out,
                 fname=self.args.file,
                 no_exec=self.args.no_exec,
-                remote=self.args.remote,
                 to_remote=self.args.to_remote,
                 desc=self.args.desc,
                 jobs=self.args.jobs,
@@ -73,14 +72,10 @@ def add_parser(subparsers, parent_parser):
     )
     import_parser.add_argument(
         "--to-remote",
-        action="store_true",
+        nargs="?",
         default=False,
+        const=True,
         help="Download it directly to the remote",
-    )
-    import_parser.add_argument(
-        "-r",
-        "--remote",
-        help="Remote storage to download to",
         metavar="<name>",
     )
     import_parser.add_argument(

dvc/repo/add.py

@@ -56,9 +56,7 @@ def add(  # noqa: C901
             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"):
+        if kwargs.get("jobs"):
             invalid_opt = "--jobs"
 
     if invalid_opt is not None:
@@ -165,11 +163,9 @@ def _process_stages(
         (out,) = stage.outs
 
         if to_remote:
+            remote = None if to_remote is True else to_remote
             out.hash_info = repo.cloud.transfer(
-                target,
-                jobs=kwargs.get("jobs"),
-                remote=kwargs.get("remote"),
-                command="add",
+                target, jobs=kwargs.get("jobs"), remote=remote, command="add",
             )
         else:
             from dvc.fs import get_cloud_fs

dvc/repo/imp_url.py

@@ -18,7 +18,6 @@ def imp_url(
     erepo=None,
     frozen=True,
     no_exec=False,
-    remote=None,
     to_remote=False,
     desc=None,
     jobs=None,
@@ -28,19 +27,14 @@ def imp_url(
 
     out = resolve_output(url, out)
     path, wdir, out = resolve_paths(
-        self, out, always_local=to_remote and not out
+        self, out, always_local=to_remote is not False and not out
     )
 
     if to_remote and no_exec:
         raise InvalidArgumentError(
             "--no-exec can't be combined with --to-remote"
         )
 
-    if not to_remote and remote:
-        raise InvalidArgumentError(
-            "--remote can't be used without --to-remote"
-        )
-
     # NOTE: when user is importing something from within their own repository
     if (
         erepo is None
@@ -74,6 +68,7 @@ def imp_url(
     if no_exec:
         stage.ignore_outs()
     elif to_remote:
+        remote = None if to_remote is True else to_remote
         stage.outs[0].hash_info = self.cloud.transfer(
             url, jobs=jobs, remote=remote, command="import-url"
         )

tests/unit/command/test_add.py

@@ -34,7 +34,6 @@ def test_add(mocker, dvc):
         fname="file",
         external=True,
         out=None,
-        remote=None,
         to_remote=False,
         desc="stage description",
         jobs=None,
@@ -43,15 +42,7 @@ def test_add(mocker, dvc):
 
 def test_add_to_remote(mocker):
     cli_args = parse_args(
-        [
-            "add",
-            "s3://bucket/foo",
-            "--to-remote",
-            "--out",
-            "bar",
-            "--remote",
-            "remote",
-        ]
+        ["add", "s3://bucket/foo", "--to-remote", "remote", "--out", "bar"]
     )
     assert cli_args.func == CmdAdd
 
@@ -68,7 +59,29 @@ def test_add_to_remote(mocker):
         fname=None,
         external=False,
         out="bar",
-        remote="remote",
+        to_remote="remote",
+        desc=None,
+        jobs=None,
+    )
+
+
+def test_add_target_after_to_remote_takes_default_remote(mocker):
+    cli_args = parse_args(["add", "--to-remote", "s3://bucket/foo"])
+    assert cli_args.func == CmdAdd
+
+    cmd = cli_args.func(cli_args)
+    m = mocker.patch.object(cmd.repo, "add", autospec=True)
+
+    assert cmd.run() == 0
+
+    m.assert_called_once_with(
+        ["s3://bucket/foo"],
+        recursive=False,
+        no_commit=False,
+        glob=False,
+        fname=None,
+        external=False,
+        out=None,
         to_remote=True,
         desc=None,
         jobs=None,
@@ -87,14 +100,14 @@ def test_add_to_remote_invalid_combinations(mocker, caplog):
         expected_msg = "multiple targets can't be used with --to-remote"
         assert expected_msg in caplog.text
 
-    for option, value in (("--remote", "remote"), ("--jobs", "4")):
-        cli_args = parse_args(["add", "foo", option, value])
+    option, value = "--jobs", "4"
+    cli_args = parse_args(["add", "foo", option, value])
 
-        cmd = cli_args.func(cli_args)
-        with caplog.at_level(logging.ERROR, logger="dvc"):
-            assert cmd.run() == 1
-            expected_msg = f"{option} can't be used without --to-remote"
-            assert expected_msg in caplog.text
+    cmd = cli_args.func(cli_args)
+    with caplog.at_level(logging.ERROR, logger="dvc"):
+        assert cmd.run() == 1
+        expected_msg = f"{option} can't be used without --to-remote"
+        assert expected_msg in caplog.text
 
 
 def test_add_to_cache_invalid_combinations(mocker, caplog):

tests/unit/command/test_imp_url.py

@@ -31,7 +31,6 @@ def test_import_url(mocker):
         out="out",
         fname="file",
         no_exec=False,
-        remote=None,
         to_remote=False,
         desc="description",
         jobs=4,
@@ -78,7 +77,6 @@ def test_import_url_no_exec(mocker):
         out="out",
         fname="file",
         no_exec=True,
-        remote=None,
         to_remote=False,
         desc="description",
         jobs=None,
@@ -92,7 +90,6 @@ def test_import_url_to_remote(mocker):
             "s3://bucket/foo",
             "bar",
             "--to-remote",
-            "--remote",
             "remote",
             "--desc",
             "description",
@@ -110,8 +107,7 @@ def test_import_url_to_remote(mocker):
         out="bar",
         fname=None,
         no_exec=False,
-        remote="remote",
-        to_remote=True,
+        to_remote="remote",
         desc="description",
         jobs=None,
     )
@@ -124,7 +120,6 @@ def test_import_url_to_remote_invalid_combination(mocker, caplog):
             "s3://bucket/foo",
             "bar",
             "--to-remote",
-            "--remote",
             "remote",
             "--no-exec",
         ]
@@ -136,13 +131,3 @@ def test_import_url_to_remote_invalid_combination(mocker, caplog):
         assert cmd.run() == 1
         expected_msg = "--no-exec can't be combined with --to-remote"
         assert expected_msg in caplog.text
-
-    cli_args = parse_args(
-        ["import-url", "s3://bucket/foo", "bar", "--remote", "remote"]
-    )
-
-    cmd = cli_args.func(cli_args)
-    with caplog.at_level(logging.ERROR, logger="dvc"):
-        assert cmd.run() == 1
-        expected_msg = "--remote can't be used without --to-remote"
-        assert expected_msg in caplog.text