Merge pull request #2053 from Suor/test-remotes

Add initial get_url()/open() api implementation

dvc/api.py

@@ -0,0 +1,15 @@
+from dvc.repo import Repo
+
+
+def get_url(path, repo_dir=None, remote=None):
+    """Returns an url of `path` in default or specified remote"""
+    repo = Repo(repo_dir)
+    out, = repo.find_outs_by_path(path)
+    remote_obj = repo.cloud.get_remote(remote)
+    return str(remote_obj.checksum_to_path_info(out.checksum))
+
+
+def open(path, repo_dir=None, remote=None, mode="r", encoding=None):
+    """Opens a specified resource as a file descriptor"""
+    repo = Repo(repo_dir)
+    return repo.open(path, remote=remote, mode=mode, encoding=encoding)

dvc/data_cloud.py

@@ -48,21 +48,25 @@ class DataCloud(object):
         self._config = config
         self._core = self._config[Config.SECTION_CORE]
 
-    @property
-    def _cloud(self):
-        """Returns a Remote instance using the `core.remote` config"""
-        remote = self._core.get(Config.SECTION_CORE_REMOTE, "")
+    def get_remote(self, remote=None, command="<command>"):
+        if not remote:
+            remote = self._core.get(Config.SECTION_CORE_REMOTE)
 
         if remote:
             return self._init_remote(remote)
 
-        if self._core.get(Config.SECTION_CORE_CLOUD, None):
-            # backward compatibility
+        # Old config format support for backward compatibility
+        if Config.SECTION_CORE_CLOUD in self._core:
             msg = "using obsoleted config format. Consider updating."
             logger.warning(msg)
             return self._init_compat()
 
-        return None
+        raise ConfigError(
+            "No remote repository specified. Setup default repository with\n"
+            "    dvc config core.remote <name>\n"
+            "or use:\n"
+            "    dvc {} -r <name>\n".format(command)
+        )
 
     def _init_remote(self, remote):
         section = Config.SECTION_REMOTE_FMT.format(remote).lower()
@@ -76,7 +80,6 @@ class DataCloud(object):
     def _init_compat(self):
         name = self._core.get(Config.SECTION_CORE_CLOUD, "").strip().lower()
         if name == "":
-            self._cloud = None
             return None
 
         cloud_type = self.CLOUD_MAP.get(name, None)
@@ -104,20 +107,6 @@ class DataCloud(object):
         cloud = cloud_type(self.repo, cloud_config)
         return cloud
 
-    def _get_cloud(self, remote, cmd):
-        if remote:
-            return self._init_remote(remote)
-
-        if self._cloud:
-            return self._cloud
-
-        raise ConfigError(
-            "No remote repository specified. Setup default repository with\n"
-            "    dvc config core.remote <name>\n"
-            "or use:\n"
-            "    dvc {} -r <name>\n".format(cmd)
-        )
-
     def push(self, targets, jobs=None, remote=None, show_checksums=False):
         """Push data items in a cloud-agnostic way.
 
@@ -132,7 +121,7 @@ class DataCloud(object):
         return self.repo.cache.local.push(
             targets,
             jobs=jobs,
-            remote=self._get_cloud(remote, "push"),
+            remote=self.get_remote(remote, "push"),
             show_checksums=show_checksums,
         )
 
@@ -150,7 +139,7 @@ class DataCloud(object):
         return self.repo.cache.local.pull(
             targets,
             jobs=jobs,
-            remote=self._get_cloud(remote, "pull"),
+            remote=self.get_remote(remote, "pull"),
             show_checksums=show_checksums,
         )
 
@@ -166,7 +155,7 @@ class DataCloud(object):
             show_checksums (bool): show checksums instead of file names in
                 information messages.
         """
-        cloud = self._get_cloud(remote, "status")
+        remote = self.get_remote(remote, "status")
         return self.repo.cache.local.status(
-            targets, jobs=jobs, remote=cloud, show_checksums=show_checksums
+            targets, jobs=jobs, remote=remote, show_checksums=show_checksums
         )

dvc/path/hdfs.py

@@ -1,4 +1,3 @@
-from dvc.utils.compat import urlunsplit
 from dvc.scheme import Schemes
 from .base import PathBASE
 
@@ -11,6 +10,4 @@ class PathHDFS(PathBASE):
         self.user = user
 
     def __str__(self):
-        if not self.url:
-            return urlunsplit((self.scheme, self.user, self.path, "", ""))
-        return self.url
+        return self.path

dvc/repo/__init__.py

@@ -13,6 +13,7 @@ from dvc.exceptions import (
 )
 from dvc.ignore import DvcIgnoreFileHandler
 from dvc.path.local import PathLOCAL
+from dvc.utils.compat import open as _open
 
 logger = logging.getLogger(__name__)
 
@@ -538,3 +539,16 @@ class Repo(object):
     def is_dvc_internal(self, path):
         path_parts = os.path.normpath(path).split(os.path.sep)
         return self.DVC_DIR in path_parts
+
+    def open(self, path, remote=None, mode="r", encoding=None):
+        """Opens a specified resource as a file descriptor"""
+        out, = self.find_outs_by_path(path)
+        if out.isdir():
+            raise ValueError("Can't open a dir")
+
+        with self.state:
+            cache_info = self._collect_used_cache(out, remote=remote)
+            self.cloud.pull(cache_info, remote=remote)
+
+        cache_filename = self.cache.local.checksum_to_path(out.checksum)
+        return _open(cache_filename, mode=mode, encoding=encoding)

dvc/repo/gc.py

@@ -110,4 +110,4 @@ def gc(
             _do_gc("azure", self.cache.azure.gc, clist)
 
         if cloud:
-            _do_gc("remote", self.cloud._get_cloud(remote, "gc -c").gc, clist)
+            _do_gc("remote", self.cloud.get_remote(remote, "gc -c").gc, clist)

tests/basic_env.py

@@ -23,9 +23,6 @@ logger = logging.getLogger("dvc")
 
 
 class TestDirFixture(object):
-    GCP_CREDS_FILE = os.path.abspath(
-        os.path.join("scripts", "ci", "gcp-creds.json")
-    )
     DATA_DIR = "data_dir"
     DATA_SUB_DIR = os.path.join(DATA_DIR, "data_sub_dir")
     DATA = os.path.join(DATA_DIR, "data")

tests/func/test_api.py

@@ -0,0 +1,93 @@
+import pytest
+import shutil
+
+from dvc import api
+from dvc.main import main
+from .test_data_cloud import (
+    _should_test_aws,
+    get_aws_url,
+    _should_test_gcp,
+    get_gcp_url,
+    _should_test_azure,
+    get_azure_url,
+    _should_test_oss,
+    get_oss_url,
+    _should_test_ssh,
+    get_ssh_url,
+    _should_test_hdfs,
+    get_hdfs_url,
+)
+
+
+# NOTE: staticmethod is only needed in Python 2
+class S3:
+    should_test = staticmethod(_should_test_aws)
+    get_url = staticmethod(get_aws_url)
+
+
+class GCP:
+    should_test = staticmethod(_should_test_gcp)
+    get_url = staticmethod(get_gcp_url)
+
+
+class Azure:
+    should_test = staticmethod(_should_test_azure)
+    get_url = staticmethod(get_azure_url)
+
+
+class OSS:
+    should_test = staticmethod(_should_test_oss)
+    get_url = staticmethod(get_oss_url)
+
+
+class SSH:
+    should_test = staticmethod(_should_test_ssh)
+    get_url = staticmethod(get_ssh_url)
+
+
+class HDFS:
+    should_test = staticmethod(_should_test_hdfs)
+    get_url = staticmethod(get_hdfs_url)
+
+
+remote_params = [S3, GCP, Azure, OSS, SSH, HDFS]
+
+
+@pytest.fixture
+def remote(request):
+    if not request.param.should_test():
+        raise pytest.skip()
+    return request.param
+
+
+def pytest_generate_tests(metafunc):
+    if "remote" in metafunc.fixturenames:
+        metafunc.parametrize("remote", remote_params, indirect=True)
+
+
+def run_dvc(*argv):
+    assert main(argv) == 0
+
+
+def test_get_url(repo_dir, dvc_repo, remote):
+    remote_url = remote.get_url()
+
+    run_dvc("remote", "add", "-d", "upstream", remote_url)
+    dvc_repo.add(repo_dir.FOO)
+
+    assert api.get_url(repo_dir.FOO) == "%s/%s" % (
+        remote_url,
+        "ac/bd18db4cc2f85cedef654fccc4a4d8",
+    )
+
+
+def test_open(repo_dir, dvc_repo, remote):
+    run_dvc("remote", "add", "-d", "upstream", remote.get_url())
+    dvc_repo.add(repo_dir.FOO)
+    run_dvc("push")
+
+    # Remove cache to force download
+    shutil.rmtree(dvc_repo.cache.local.cache_dir)
+
+    with api.open(repo_dir.FOO) as fd:
+        assert fd.read() == repo_dir.FOO_CONTENTS

tests/func/test_data_cloud.py

@@ -47,6 +47,15 @@ TEST_AWS_REPO_BUCKET = os.environ.get("DVC_TEST_AWS_REPO_BUCKET", "dvc-test")
 TEST_GCP_REPO_BUCKET = os.environ.get("DVC_TEST_GCP_REPO_BUCKET", "dvc-test")
 TEST_OSS_REPO_BUCKET = "dvc-test"
 
+TEST_GCP_CREDS_FILE = os.path.abspath(
+    os.environ.get(
+        "GOOGLE_APPLICATION_CREDENTIALS",
+        os.path.join("scripts", "ci", "gcp-creds.json"),
+    )
+)
+# Ensure that absolute path is used
+os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = TEST_GCP_CREDS_FILE
+
 
 def _should_test_aws():
     dvc_test_aws = os.getenv("DVC_TEST_AWS")
@@ -65,31 +74,22 @@ def _should_test_gcp():
     if os.getenv("DVC_TEST_GCP") == "true":
         return True
 
-    if not os.path.exists(TestDvc.GCP_CREDS_FILE):
+    if not os.path.exists(TEST_GCP_CREDS_FILE):
         return False
 
-    # NOTE: we copy because GOOGLE_APPLICATION_CREDENTIALS uses rel path.
-    #       This is only my guess and I don't know why won't we use absolute.
-    creds = os.getenv("GOOGLE_APPLICATION_CREDENTIALS")
-    if creds and os.getenv("GCP_CREDS"):
-        if os.path.exists(creds):
-            os.unlink(creds)
-        shutil.copyfile(TestDvc.GCP_CREDS_FILE, creds)
-        try:
-            check_output(
-                [
-                    "gcloud",
-                    "auth",
-                    "activate-service-account",
-                    "--key-file",
-                    creds,
-                ]
-            )
-        except (CalledProcessError, OSError):
-            return False
-        return True
-
-    return False
+    try:
+        check_output(
+            [
+                "gcloud",
+                "auth",
+                "activate-service-account",
+                "--key-file",
+                TEST_GCP_CREDS_FILE,
+            ]
+        )
+    except (CalledProcessError, OSError):
+        return False
+    return True
 
 
 def _should_test_azure():
@@ -164,7 +164,7 @@ def get_local_url():
 
 
 def get_ssh_url():
-    return "ssh://{}@127.0.0.1:{}".format(
+    return "ssh://{}@127.0.0.1:22{}".format(
         getpass.getuser(), get_local_storagepath()
     )
 
@@ -236,7 +236,7 @@ def get_oss_url():
 class TestDataCloud(TestDvc):
     def _test_cloud(self, config, cl):
         cloud = DataCloud(self.dvc, config=config)
-        self.assertIsInstance(cloud._cloud, cl)
+        self.assertIsInstance(cloud.get_remote(), cl)
 
     def test(self):
         config = copy.deepcopy(TEST_CONFIG)
@@ -287,7 +287,7 @@ class TestDataCloudBase(TestDvc):
         config[TEST_SECTION][Config.SECTION_REMOTE_KEY_FILE] = keyfile
         self.cloud = DataCloud(self.dvc, config)
 
-        self.assertIsInstance(self.cloud._cloud, self._get_cloud_class())
+        self.assertIsInstance(self.cloud.get_remote(), self._get_cloud_class())
 
     def _test_cloud(self):
         self._setup_cloud()
@@ -393,10 +393,10 @@ class TestRemoteGS(TestDataCloudBase):
         config[TEST_SECTION][Config.SECTION_REMOTE_URL] = repo
         config[TEST_SECTION][
             Config.SECTION_GCP_CREDENTIALPATH
-        ] = TestDvc.GCP_CREDS_FILE
+        ] = TEST_GCP_CREDS_FILE
         self.cloud = DataCloud(self.dvc, config)
 
-        self.assertIsInstance(self.cloud._cloud, self._get_cloud_class())
+        self.assertIsInstance(self.cloud.get_remote(), self._get_cloud_class())
 
     def _get_url(self):
         return get_gcp_url()
@@ -668,7 +668,7 @@ class TestRemoteGSCLI(TestDataCloudCLIBase):
                 "modify",
                 TEST_REMOTE,
                 "credentialpath",
-                TestDvc.GCP_CREDS_FILE,
+                TEST_GCP_CREDS_FILE,
             ]
         )
 
@@ -760,9 +760,8 @@ class TestRecursiveSyncOperations(TestDataCloudBase):
         return RemoteLOCAL
 
     def _prepare_repo(self):
-        self.main(
-            ["remote", "add", "-d", TEST_REMOTE, self.cloud._cloud.cache_dir]
-        )
+        remote = self.cloud.get_remote()
+        self.main(["remote", "add", "-d", TEST_REMOTE, remote.cache_dir])
 
         self.dvc.add(self.DATA)
         self.dvc.add(self.DATA_SUB)
@@ -801,8 +800,9 @@ class TestRecursiveSyncOperations(TestDataCloudBase):
         self.assertTrue(os.path.exists(local_cache_data_sub_path))
 
     def _test_recursive_push(self, data_md5, data_sub_md5):
-        cloud_data_path = self.cloud._cloud.get(data_md5)
-        cloud_data_sub_path = self.cloud._cloud.get(data_sub_md5)
+        remote = self.cloud.get_remote()
+        cloud_data_path = remote.get(data_md5)
+        cloud_data_sub_path = remote.get(data_sub_md5)
 
         self.assertFalse(os.path.exists(cloud_data_path))
         self.assertFalse(os.path.exists(cloud_data_sub_path))

tests/remotes_env.sample

@@ -4,9 +4,7 @@
 # export DVC_TEST_AWS_REPO_BUCKET="...SET-ME..."
 
 # Uncomment and set a bucket name to test against Google Storage
-# export GOOGLE_APPLICATION_CREDENTIALS=".gcp-creds.json"
-# export GCP_CREDS="yes"
-# export DVC_TEST_GCP_REPO_BUCKET="...SET-ME..."
+# export GOOGLE_APPLICATION_CREDENTIALS="scripts/ci/gcp-creds.json"
 
 # Uncomment to test against Microsoft Azure via Azurite
 # export AZURE_STORAGE_CONTAINER_NAME="dvc-test"