From 98750c95c506a2ad5cc3e86f5014a0a4ca3313cb Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Thu, 31 Oct 2019 09:58:07 -0700 Subject: [PATCH 1/2] get-url: fix NPE when src non local and dst not absolute --- dvc/output/local.py | 3 ++- dvc/repo/get_url.py | 3 ++- tests/func/test_get_url.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/dvc/output/local.py b/dvc/output/local.py index 777ac37569..ea0cb9d18b 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -26,9 +26,10 @@ def _parse_path(self, remote, path): # so we should expect both posix and windows style paths. # PathInfo accepts both, i.e. / works everywhere, \ only on win. # - # FIXME: if we have Windows path containig / or posix one with \ + # FIXME: if we have Windows path containing / or posix one with \ # then we have #2059 bug and can't really handle that. p = self.REMOTE.path_cls(path) + if not p.is_absolute(): p = self.stage.wdir / p diff --git a/dvc/repo/get_url.py b/dvc/repo/get_url.py index ff191e149e..8ba1878653 100644 --- a/dvc/repo/get_url.py +++ b/dvc/repo/get_url.py @@ -12,7 +12,8 @@ def get_url(url, out=None): if os.path.exists(url): url = os.path.abspath(url) - out = os.path.abspath(out) + + out = os.path.abspath(out) dep, = dependency.loads_from(None, [url]) out, = output.loads_from(None, [out], use_cache=False) diff --git a/tests/func/test_get_url.py b/tests/func/test_get_url.py index eae2b556ef..fab7a4db67 100644 --- a/tests/func/test_get_url.py +++ b/tests/func/test_get_url.py @@ -1,13 +1,19 @@ from __future__ import unicode_literals import os +import boto3 import filecmp import pytest +from moto import mock_s3 + +from dvc.remote import RemoteS3 from dvc.repo import Repo from dvc.utils import makedirs +from tests.func.test_data_cloud import get_aws_url + def test_get_file(repo_dir): src = repo_dir.FOO @@ -32,3 +38,27 @@ def test_get_url_to_dir(dname, repo_dir): assert os.path.isdir(dname) assert filecmp.cmp(repo_dir.DATA, dst, shallow=False) + + +@mock_s3 +@pytest.mark.parametrize("dst", [".", "./from"]) +def test_get_url_from_non_local_path_to_dir_and_file(repo_dir, dst): + file_name = "from" + file_content = "data" + base_info = RemoteS3.path_cls(get_aws_url()) + from_info = base_info / file_name + + s3 = boto3.client("s3") + s3.create_bucket(Bucket=from_info.bucket) + s3.put_object( + Bucket=from_info.bucket, Key=from_info.path, Body=file_content + ) + + Repo.get_url(from_info.url, dst) + + result_path = os.path.join(dst, file_name) if os.path.isdir(dst) else dst + + assert os.path.exists(result_path) + assert os.path.isfile(result_path) + with open(result_path, "r") as fd: + assert fd.read() == file_content From 3660d9eac470a48e152be8dcbf65058976edb8bb Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Fri, 1 Nov 2019 09:49:56 -0700 Subject: [PATCH 2/2] local output: address PR comment, fix style a bit --- dvc/output/local.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dvc/output/local.py b/dvc/output/local.py index ea0cb9d18b..cbd9c9503b 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -29,7 +29,6 @@ def _parse_path(self, remote, path): # FIXME: if we have Windows path containing / or posix one with \ # then we have #2059 bug and can't really handle that. p = self.REMOTE.path_cls(path) - if not p.is_absolute(): p = self.stage.wdir / p