Skip to content

Commit

Permalink
add, import-url: always resolve the output in the local path for --to…
Browse files Browse the repository at this point in the history
…-remote
  • Loading branch information
isidentical committed Feb 16, 2021
1 parent 53ba51b commit b457d98
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 7 deletions.
14 changes: 12 additions & 2 deletions dvc/repo/add.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ def add( # noqa: C901
invalid_opt = "--no-commit option"
elif recursive:
invalid_opt = "--recursive option"
elif kwargs.get("external"):
invalid_opt = "--external option"
else:
message = "{option} can't be used without --to-remote"
if kwargs.get("remote"):
Expand Down Expand Up @@ -88,7 +90,12 @@ def add( # noqa: C901
)

stages = _create_stages(
repo, sub_targets, fname, pbar=pbar, **kwargs,
repo,
sub_targets,
fname,
pbar=pbar,
transfer=to_remote or to_cache,
**kwargs,
)

try:
Expand Down Expand Up @@ -230,6 +237,7 @@ def _create_stages(
external=False,
glob=False,
desc=None,
transfer=False,
**kwargs,
):
from dvc.dvcfile import Dvcfile
Expand All @@ -246,7 +254,9 @@ def _create_stages(
):
if kwargs.get("out"):
out = resolve_output(out, kwargs["out"])
path, wdir, out = resolve_paths(repo, out)
path, wdir, out = resolve_paths(
repo, out, always_local=transfer and not kwargs.get("out")
)
stage = create_stage(
Stage,
repo,
Expand Down
4 changes: 3 additions & 1 deletion dvc/repo/imp_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ def imp_url(
from dvc.stage import Stage, create_stage, restore_meta

out = resolve_output(url, out)
path, wdir, out = resolve_paths(self, out)
path, wdir, out = resolve_paths(
self, out, always_local=to_remote and not out
)

if to_remote and no_exec:
raise InvalidArgumentError(
Expand Down
5 changes: 4 additions & 1 deletion dvc/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ def resolve_output(inp, out):
return ret


def resolve_paths(repo, out):
def resolve_paths(repo, out, always_local=False):
from urllib.parse import urlparse

from ..dvcfile import DVC_FILE_SUFFIX
Expand Down Expand Up @@ -393,6 +393,9 @@ def resolve_paths(repo, out):
wdir = dirname
out = base

if always_local:
out = base

path = os.path.join(wdir, base + DVC_FILE_SUFFIX)

return (path, wdir, out)
Expand Down
35 changes: 32 additions & 3 deletions tests/func/test_add.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import stat
import textwrap
import time
from tempfile import NamedTemporaryFile

import colorama
import pytest
Expand All @@ -27,7 +28,10 @@
from dvc.output.base import OutputAlreadyTrackedError, OutputIsStageFileError
from dvc.repo import Repo as DvcRepo
from dvc.stage import Stage
from dvc.stage.exceptions import StagePathNotFoundError
from dvc.stage.exceptions import (
StageExternalOutputsError,
StagePathNotFoundError,
)
from dvc.system import System
from dvc.utils import LARGE_DIR_SIZE, file_md5, relpath
from dvc.utils.fs import path_isin
Expand Down Expand Up @@ -324,8 +328,6 @@ def test_add_filtered_files_in_dir(
indirect=["workspace"],
)
def test_add_external_file(tmp_dir, dvc, workspace, hash_name, hash_value):
from dvc.stage.exceptions import StageExternalOutputsError

workspace.gen("file", "file")

with pytest.raises(StageExternalOutputsError):
Expand Down Expand Up @@ -1013,12 +1015,39 @@ def test_add_to_remote(tmp_dir, dvc, local_cloud, local_remote):
assert local_remote.hash_to_path_info(hash_info.value).read_text() == "foo"


def test_add_to_remote_absolute(tmp_dir, dvc, local_remote):
with NamedTemporaryFile("w") as temp_file:
temp_file.write("foo")
temp_file.file.close()

base = os.path.basename(temp_file.name)
dvc.add(temp_file.name, to_remote=True)

assert (tmp_dir / base).with_suffix(".dvc").exists()
assert not os.path.exists(temp_file.name)

dvc.pull(base)
assert not os.path.exists(temp_file.name)
assert (tmp_dir / base).read_text() == "foo"

with pytest.raises(StageExternalOutputsError):
with NamedTemporaryFile("w") as temp_file:
temp_file.write("foo")
temp_file.file.close()

base = os.path.basename(temp_file.name)
dvc.add(
temp_file.name, out=temp_file.name + "_new", to_remote=True
)


@pytest.mark.parametrize(
"invalid_opt, kwargs",
[
("multiple targets", {"targets": ["foo", "bar", "baz"]}),
("--no-commit", {"targets": ["foo"], "no_commit": True}),
("--recursive", {"targets": ["foo"], "recursive": True},),
("--external", {"targets": ["foo"], "external": True}),
],
)
def test_add_to_remote_invalid_combinations(dvc, invalid_opt, kwargs):
Expand Down
15 changes: 15 additions & 0 deletions tests/func/test_import_url.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
import os
import textwrap
from tempfile import NamedTemporaryFile
from uuid import uuid4

import pytest
Expand Down Expand Up @@ -340,6 +341,20 @@ def test_import_url_to_remote_directory(tmp_dir, dvc, workspace, local_remote):
)


def test_import_url_to_remote_absolute(tmp_dir, dvc, local_remote):
with NamedTemporaryFile("w") as temp_file:
temp_file.write("foo")
temp_file.file.close()

base = os.path.basename(temp_file.name)
stage = dvc.imp_url(temp_file.name, to_remote=True)

base_info = tmp_dir / base
assert stage.deps[0].fspath == temp_file.name
assert stage.outs[0].fspath == os.fspath(base_info)
assert base_info.with_suffix(".dvc").exists()


def test_import_url_to_remote_invalid_combinations(dvc):
with pytest.raises(InvalidArgumentError, match="--no-exec"):
dvc.imp_url("s3://bucket/foo", no_exec=True, to_remote=True)

0 comments on commit b457d98

Please sign in to comment.