Skip to content

Commit 7c84960

Browse files
committed
ui: merge --to-remote and --remote for add and import-url
Fixes #5719
1 parent 7d1a353 commit 7c84960

File tree

6 files changed

+19
-63
lines changed

6 files changed

+19
-63
lines changed

dvc/command/add.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ def run(self):
2727
glob=self.args.glob,
2828
desc=self.args.desc,
2929
out=self.args.out,
30-
remote=self.args.remote,
3130
to_remote=self.args.to_remote,
3231
jobs=self.args.jobs,
3332
)
@@ -86,14 +85,10 @@ def add_parser(subparsers, parent_parser):
8685
)
8786
parser.add_argument(
8887
"--to-remote",
89-
action="store_true",
88+
nargs="?",
9089
default=False,
90+
const=True,
9191
help="Download it directly to the remote",
92-
)
93-
parser.add_argument(
94-
"-r",
95-
"--remote",
96-
help="Remote storage to download to",
9792
metavar="<name>",
9893
)
9994
parser.add_argument(

dvc/command/imp_url.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ def run(self):
1616
out=self.args.out,
1717
fname=self.args.file,
1818
no_exec=self.args.no_exec,
19-
remote=self.args.remote,
2019
to_remote=self.args.to_remote,
2120
desc=self.args.desc,
2221
jobs=self.args.jobs,
@@ -73,14 +72,10 @@ def add_parser(subparsers, parent_parser):
7372
)
7473
import_parser.add_argument(
7574
"--to-remote",
76-
action="store_true",
75+
nargs="?",
7776
default=False,
77+
const=True,
7878
help="Download it directly to the remote",
79-
)
80-
import_parser.add_argument(
81-
"-r",
82-
"--remote",
83-
help="Remote storage to download to",
8479
metavar="<name>",
8580
)
8681
import_parser.add_argument(

dvc/repo/add.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,7 @@ def add( # noqa: C901
5656
invalid_opt = "--external option"
5757
else:
5858
message = "{option} can't be used without --to-remote"
59-
if kwargs.get("remote"):
60-
invalid_opt = "--remote"
61-
elif kwargs.get("jobs"):
59+
if kwargs.get("jobs"):
6260
invalid_opt = "--jobs"
6361

6462
if invalid_opt is not None:
@@ -165,11 +163,9 @@ def _process_stages(
165163
(out,) = stage.outs
166164

167165
if to_remote:
166+
remote = None if to_remote is True else to_remote
168167
out.hash_info = repo.cloud.transfer(
169-
target,
170-
jobs=kwargs.get("jobs"),
171-
remote=kwargs.get("remote"),
172-
command="add",
168+
target, jobs=kwargs.get("jobs"), remote=remote, command="add",
173169
)
174170
else:
175171
from dvc.fs import get_cloud_fs

dvc/repo/imp_url.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ def imp_url(
1818
erepo=None,
1919
frozen=True,
2020
no_exec=False,
21-
remote=None,
2221
to_remote=False,
2322
desc=None,
2423
jobs=None,
@@ -28,19 +27,14 @@ def imp_url(
2827

2928
out = resolve_output(url, out)
3029
path, wdir, out = resolve_paths(
31-
self, out, always_local=to_remote and not out
30+
self, out, always_local=to_remote is not False and not out
3231
)
3332

3433
if to_remote and no_exec:
3534
raise InvalidArgumentError(
3635
"--no-exec can't be combined with --to-remote"
3736
)
3837

39-
if not to_remote and remote:
40-
raise InvalidArgumentError(
41-
"--remote can't be used without --to-remote"
42-
)
43-
4438
# NOTE: when user is importing something from within their own repository
4539
if (
4640
erepo is None
@@ -74,6 +68,7 @@ def imp_url(
7468
if no_exec:
7569
stage.ignore_outs()
7670
elif to_remote:
71+
remote = None if to_remote is True else to_remote
7772
stage.outs[0].hash_info = self.cloud.transfer(
7873
url, jobs=jobs, remote=remote, command="import-url"
7974
)

tests/unit/command/test_add.py

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ def test_add(mocker, dvc):
3434
fname="file",
3535
external=True,
3636
out=None,
37-
remote=None,
3837
to_remote=False,
3938
desc="stage description",
4039
jobs=None,
@@ -43,15 +42,7 @@ def test_add(mocker, dvc):
4342

4443
def test_add_to_remote(mocker):
4544
cli_args = parse_args(
46-
[
47-
"add",
48-
"s3://bucket/foo",
49-
"--to-remote",
50-
"--out",
51-
"bar",
52-
"--remote",
53-
"remote",
54-
]
45+
["add", "s3://bucket/foo", "--to-remote", "remote", "--out", "bar"]
5546
)
5647
assert cli_args.func == CmdAdd
5748

@@ -68,8 +59,7 @@ def test_add_to_remote(mocker):
6859
fname=None,
6960
external=False,
7061
out="bar",
71-
remote="remote",
72-
to_remote=True,
62+
to_remote="remote",
7363
desc=None,
7464
jobs=None,
7565
)
@@ -87,14 +77,14 @@ def test_add_to_remote_invalid_combinations(mocker, caplog):
8777
expected_msg = "multiple targets can't be used with --to-remote"
8878
assert expected_msg in caplog.text
8979

90-
for option, value in (("--remote", "remote"), ("--jobs", "4")):
91-
cli_args = parse_args(["add", "foo", option, value])
80+
option, value = "--jobs", "4"
81+
cli_args = parse_args(["add", "foo", option, value])
9282

93-
cmd = cli_args.func(cli_args)
94-
with caplog.at_level(logging.ERROR, logger="dvc"):
95-
assert cmd.run() == 1
96-
expected_msg = f"{option} can't be used without --to-remote"
97-
assert expected_msg in caplog.text
83+
cmd = cli_args.func(cli_args)
84+
with caplog.at_level(logging.ERROR, logger="dvc"):
85+
assert cmd.run() == 1
86+
expected_msg = f"{option} can't be used without --to-remote"
87+
assert expected_msg in caplog.text
9888

9989

10090
def test_add_to_cache_invalid_combinations(mocker, caplog):

tests/unit/command/test_imp_url.py

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ def test_import_url(mocker):
3131
out="out",
3232
fname="file",
3333
no_exec=False,
34-
remote=None,
3534
to_remote=False,
3635
desc="description",
3736
jobs=4,
@@ -78,7 +77,6 @@ def test_import_url_no_exec(mocker):
7877
out="out",
7978
fname="file",
8079
no_exec=True,
81-
remote=None,
8280
to_remote=False,
8381
desc="description",
8482
jobs=None,
@@ -92,7 +90,6 @@ def test_import_url_to_remote(mocker):
9290
"s3://bucket/foo",
9391
"bar",
9492
"--to-remote",
95-
"--remote",
9693
"remote",
9794
"--desc",
9895
"description",
@@ -110,8 +107,7 @@ def test_import_url_to_remote(mocker):
110107
out="bar",
111108
fname=None,
112109
no_exec=False,
113-
remote="remote",
114-
to_remote=True,
110+
to_remote="remote",
115111
desc="description",
116112
jobs=None,
117113
)
@@ -124,7 +120,6 @@ def test_import_url_to_remote_invalid_combination(mocker, caplog):
124120
"s3://bucket/foo",
125121
"bar",
126122
"--to-remote",
127-
"--remote",
128123
"remote",
129124
"--no-exec",
130125
]
@@ -136,13 +131,3 @@ def test_import_url_to_remote_invalid_combination(mocker, caplog):
136131
assert cmd.run() == 1
137132
expected_msg = "--no-exec can't be combined with --to-remote"
138133
assert expected_msg in caplog.text
139-
140-
cli_args = parse_args(
141-
["import-url", "s3://bucket/foo", "bar", "--remote", "remote"]
142-
)
143-
144-
cmd = cli_args.func(cli_args)
145-
with caplog.at_level(logging.ERROR, logger="dvc"):
146-
assert cmd.run() == 1
147-
expected_msg = "--remote can't be used without --to-remote"
148-
assert expected_msg in caplog.text

0 commit comments

Comments
 (0)