From 0dd310d3873ceb55096652f3f08eab90a6e381f1 Mon Sep 17 00:00:00 2001 From: Mayur Kulkarni Date: Sat, 7 Dec 2019 18:39:32 +0530 Subject: [PATCH 01/14] external_repo: fix for local file import fail If the URL is local dvc repo, the import/get should fetch from the source project cache. Fixes #2599 --- dvc/external_repo.py | 49 +++++++++++++++++++++++++++++++- tests/func/test_external_repo.py | 9 ++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 0555f2d756..cbd33b250d 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -5,9 +5,10 @@ from contextlib import contextmanager from distutils.dir_util import copy_tree +from dvc.remote import RemoteConfig from funcy import retry -from dvc.config import NoRemoteError +from dvc.config import NoRemoteError, ConfigError from dvc.exceptions import RemoteNotSpecifiedInExternalRepoError from dvc.exceptions import NoOutputInExternalRepoError from dvc.exceptions import OutputNotFoundError @@ -62,16 +63,29 @@ def _external_repo(url=None, rev=None, cache_dir=None): # Adjust new clone/copy to fit rev and cache_dir repo = Repo(new_path) + # Adjust original repo for pointing remote towards its' cache + original_repo = Repo(url) + rconfig = RemoteConfig(original_repo.config) try: if rev is not None: repo.scm.checkout(rev) + if not _is_local(url) and not _remote_config_exists(rconfig): + # check if the URL is local and no default remote + # add default remote pointing to the original repo's cache location + rconfig.add("upstream", + original_repo.cache.local.cache_dir, + default=True) + original_repo.scm.add([original_repo.config.config_file]) + original_repo.scm.commit("add remote") + if cache_dir is not None: cache_config = CacheConfig(repo.config) cache_config.set_dir(cache_dir, level=Config.LEVEL_LOCAL) finally: # Need to close/reopen repo to force config reread repo.close() + original_repo.close() REPO_CACHE[key] = new_path return new_path @@ -100,3 +114,36 @@ def _clone_repo(url, path): git = Git.clone(url, path) git.close() + + +def _remote_config_exists(rconfig): + """ + Checks if default remote config is present. + Args: + rconfig: a remote config + + Returns: + True if the remote config exists, else False + """ + try: + default = rconfig.get_default() + except ConfigError: + default = None + return True if default else False + + +def _is_local(url): + """ + Checks if the URL is local or not. + Args: + url: url + + Returns: + True, if the URL is local else False + """ + remote_urls = {"azure://", "gs://", "http://", "https://", + "oss://", "s3://", "hdfs://"} + for remote_url in remote_urls: + if url.startswith(remote_url): + return False + return True diff --git a/tests/func/test_external_repo.py b/tests/func/test_external_repo.py index de2b8b20bb..d21e056a9d 100644 --- a/tests/func/test_external_repo.py +++ b/tests/func/test_external_repo.py @@ -1,5 +1,6 @@ import os +from dvc.repo import Repo from mock import patch from dvc.external_repo import external_repo @@ -26,3 +27,11 @@ def test_external_repo(erepo): assert path_isin(repo.cache.local.cache_dir, repo.root_dir) assert mock.call_count == 1 + + +def test_external_repo_import_without_remote(erepo, dvc_repo): + src = erepo.CODE + dst = dvc_repo.root_dir + + Repo.get(erepo.root_dir, src, dst) + assert os.path.exists(dst + "/" + erepo.CODE) From 9e24be14f0cfa588db1fa1b18085581d742958cd Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Sun, 8 Dec 2019 14:02:44 +0530 Subject: [PATCH 02/14] incorporated feedback --- dvc/external_repo.py | 47 ++++++++++++++++---------------------------- tests/conftest.py | 6 ------ 2 files changed, 17 insertions(+), 36 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index cbd33b250d..1216e836c1 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -63,21 +63,24 @@ def _external_repo(url=None, rev=None, cache_dir=None): # Adjust new clone/copy to fit rev and cache_dir repo = Repo(new_path) - # Adjust original repo for pointing remote towards its' cache - original_repo = Repo(url) - rconfig = RemoteConfig(original_repo.config) + original_repo = None try: if rev is not None: repo.scm.checkout(rev) - - if not _is_local(url) and not _remote_config_exists(rconfig): - # check if the URL is local and no default remote - # add default remote pointing to the original repo's cache location - rconfig.add("upstream", - original_repo.cache.local.cache_dir, - default=True) - original_repo.scm.add([original_repo.config.config_file]) - original_repo.scm.commit("add remote") + # check if the URL is local and no default remote + # add default remote pointing to the original repo's cache location + if not os.path.isdir(url): + # Adjust original repo for pointing remote towards its' cache + original_repo = Repo(url) + rconfig = RemoteConfig(original_repo.config) + if not _remote_config_exists(rconfig): + rconfig.add( + "upstream", + original_repo.cache.local.cache_dir, + default=True, + ) + original_repo.scm.add([original_repo.config.config_file]) + original_repo.scm.commit("add remote") if cache_dir is not None: cache_config = CacheConfig(repo.config) @@ -85,7 +88,8 @@ def _external_repo(url=None, rev=None, cache_dir=None): finally: # Need to close/reopen repo to force config reread repo.close() - original_repo.close() + if original_repo: + original_repo.close() REPO_CACHE[key] = new_path return new_path @@ -130,20 +134,3 @@ def _remote_config_exists(rconfig): except ConfigError: default = None return True if default else False - - -def _is_local(url): - """ - Checks if the URL is local or not. - Args: - url: url - - Returns: - True, if the URL is local else False - """ - remote_urls = {"azure://", "gs://", "http://", "https://", - "oss://", "s3://", "hdfs://"} - for remote_url in remote_urls: - if url.startswith(remote_url): - return False - return True diff --git a/tests/conftest.py b/tests/conftest.py index 0cdb204b88..730b6d32d5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,7 +10,6 @@ from .basic_env import TestDirFixture from .basic_env import TestDvcGitFixture from .basic_env import TestGitFixture -from dvc.remote.config import RemoteConfig from dvc.remote.ssh.connection import SSHConnection from dvc.repo import Repo as DvcRepo from dvc.utils.compat import cast_bytes_py2 @@ -148,11 +147,6 @@ def erepo(repo_dir): repo.dvc.scm.add([stage_foo.path, stage_bar.path, stage_data_dir.path]) repo.dvc.scm.commit("init repo") - rconfig = RemoteConfig(repo.dvc.config) - rconfig.add("upstream", repo.dvc.cache.local.cache_dir, default=True) - repo.dvc.scm.add([repo.dvc.config.config_file]) - repo.dvc.scm.commit("add remote") - repo.create("version", "master") repo.dvc.add("version") repo.dvc.scm.add([".gitignore", "version.dvc"]) From 779a0a1b15c9722263695f032ea95ce757291b60 Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Sun, 8 Dec 2019 14:35:51 +0530 Subject: [PATCH 03/14] codeclimate recommendation fix --- dvc/external_repo.py | 5 ++--- tests/func/test_external_repo.py | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 1216e836c1..2b24735b6f 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -4,10 +4,9 @@ import tempfile from contextlib import contextmanager from distutils.dir_util import copy_tree - -from dvc.remote import RemoteConfig from funcy import retry +from dvc.remote import RemoteConfig from dvc.config import NoRemoteError, ConfigError from dvc.exceptions import RemoteNotSpecifiedInExternalRepoError from dvc.exceptions import NoOutputInExternalRepoError @@ -133,4 +132,4 @@ def _remote_config_exists(rconfig): default = rconfig.get_default() except ConfigError: default = None - return True if default else False + return bool(default) diff --git a/tests/func/test_external_repo.py b/tests/func/test_external_repo.py index d21e056a9d..e6a8ee7bce 100644 --- a/tests/func/test_external_repo.py +++ b/tests/func/test_external_repo.py @@ -1,8 +1,8 @@ import os -from dvc.repo import Repo from mock import patch +from dvc.repo import Repo from dvc.external_repo import external_repo from dvc.scm.git import Git from dvc.utils.fs import path_isin From e1c5f6c5c062052d615eb3766bb81abcc2de3179 Mon Sep 17 00:00:00 2001 From: Mayur Kulkarni Date: Sat, 7 Dec 2019 18:39:32 +0530 Subject: [PATCH 04/14] external_repo: fix for local file import fail If the URL is local dvc repo, the import/get should fetch from the source project cache. Fixes #2599 --- dvc/external_repo.py | 50 +++++++++++++++++++++++++++++++- tests/func/test_external_repo.py | 9 ++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 8f53da4ef8..61b4a85f39 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -5,10 +5,12 @@ from contextlib import contextmanager from distutils.dir_util import copy_tree +from dvc.remote import RemoteConfig from funcy import retry -from dvc.config import NoRemoteError + from dvc.exceptions import NoRemoteInExternalRepoError +from dvc.config import NoRemoteError, ConfigError from dvc.exceptions import NoOutputInExternalRepoError from dvc.exceptions import OutputNotFoundError from dvc.utils.fs import remove @@ -68,13 +70,26 @@ def _external_repo(url=None, rev=None, cache_dir=None): _git_checkout(new_path, rev) repo = Repo(new_path) + # Adjust original repo for pointing remote towards its' cache + original_repo = Repo(url) + rconfig = RemoteConfig(original_repo.config) try: + if not _is_local(url) and not _remote_config_exists(rconfig): + # check if the URL is local and no default remote + # add default remote pointing to the original repo's cache location + rconfig.add("upstream", + original_repo.cache.local.cache_dir, + default=True) + original_repo.scm.add([original_repo.config.config_file]) + original_repo.scm.commit("add remote") + if cache_dir is not None: cache_config = CacheConfig(repo.config) cache_config.set_dir(cache_dir, level=Config.LEVEL_LOCAL) finally: # Need to close/reopen repo to force config reread repo.close() + original_repo.close() REPO_CACHE[key] = new_path return new_path @@ -113,3 +128,36 @@ def _clone_repo(url, path): git = Git.clone(url, path) git.close() + + +def _remote_config_exists(rconfig): + """ + Checks if default remote config is present. + Args: + rconfig: a remote config + + Returns: + True if the remote config exists, else False + """ + try: + default = rconfig.get_default() + except ConfigError: + default = None + return True if default else False + + +def _is_local(url): + """ + Checks if the URL is local or not. + Args: + url: url + + Returns: + True, if the URL is local else False + """ + remote_urls = {"azure://", "gs://", "http://", "https://", + "oss://", "s3://", "hdfs://"} + for remote_url in remote_urls: + if url.startswith(remote_url): + return False + return True diff --git a/tests/func/test_external_repo.py b/tests/func/test_external_repo.py index de2b8b20bb..d21e056a9d 100644 --- a/tests/func/test_external_repo.py +++ b/tests/func/test_external_repo.py @@ -1,5 +1,6 @@ import os +from dvc.repo import Repo from mock import patch from dvc.external_repo import external_repo @@ -26,3 +27,11 @@ def test_external_repo(erepo): assert path_isin(repo.cache.local.cache_dir, repo.root_dir) assert mock.call_count == 1 + + +def test_external_repo_import_without_remote(erepo, dvc_repo): + src = erepo.CODE + dst = dvc_repo.root_dir + + Repo.get(erepo.root_dir, src, dst) + assert os.path.exists(dst + "/" + erepo.CODE) From ca20a1c93aed85562f4b8e11fb7dde42c3cee73f Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Sun, 8 Dec 2019 14:02:44 +0530 Subject: [PATCH 05/14] incorporated feedback --- dvc/external_repo.py | 43 ++++++++++++++----------------------------- tests/conftest.py | 6 ------ 2 files changed, 14 insertions(+), 35 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 61b4a85f39..e4f6586b13 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -70,18 +70,21 @@ def _external_repo(url=None, rev=None, cache_dir=None): _git_checkout(new_path, rev) repo = Repo(new_path) - # Adjust original repo for pointing remote towards its' cache - original_repo = Repo(url) - rconfig = RemoteConfig(original_repo.config) try: - if not _is_local(url) and not _remote_config_exists(rconfig): - # check if the URL is local and no default remote - # add default remote pointing to the original repo's cache location - rconfig.add("upstream", - original_repo.cache.local.cache_dir, - default=True) - original_repo.scm.add([original_repo.config.config_file]) - original_repo.scm.commit("add remote") + # check if the URL is local and no default remote + # add default remote pointing to the original repo's cache location + if not os.path.isdir(url): + # Adjust original repo for pointing remote towards its' cache + original_repo = Repo(url) + rconfig = RemoteConfig(original_repo.config) + if not _remote_config_exists(rconfig): + rconfig.add( + "upstream", + original_repo.cache.local.cache_dir, + default=True, + ) + original_repo.scm.add([original_repo.config.config_file]) + original_repo.scm.commit("add remote") if cache_dir is not None: cache_config = CacheConfig(repo.config) @@ -89,7 +92,6 @@ def _external_repo(url=None, rev=None, cache_dir=None): finally: # Need to close/reopen repo to force config reread repo.close() - original_repo.close() REPO_CACHE[key] = new_path return new_path @@ -144,20 +146,3 @@ def _remote_config_exists(rconfig): except ConfigError: default = None return True if default else False - - -def _is_local(url): - """ - Checks if the URL is local or not. - Args: - url: url - - Returns: - True, if the URL is local else False - """ - remote_urls = {"azure://", "gs://", "http://", "https://", - "oss://", "s3://", "hdfs://"} - for remote_url in remote_urls: - if url.startswith(remote_url): - return False - return True diff --git a/tests/conftest.py b/tests/conftest.py index 46b059d5ef..3eca524c07 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,7 +7,6 @@ from git import Repo from git.exc import GitCommandNotFound -from dvc.remote.config import RemoteConfig from dvc.remote.ssh.connection import SSHConnection from dvc.repo import Repo as DvcRepo from dvc.utils.compat import cast_bytes_py2 @@ -148,11 +147,6 @@ def erepo(repo_dir): repo.dvc.scm.add([stage_foo.path, stage_bar.path, stage_data_dir.path]) repo.dvc.scm.commit("init repo") - rconfig = RemoteConfig(repo.dvc.config) - rconfig.add("upstream", repo.dvc.cache.local.cache_dir, default=True) - repo.dvc.scm.add([repo.dvc.config.config_file]) - repo.dvc.scm.commit("add remote") - repo.create("version", "master") repo.dvc.add("version") repo.dvc.scm.add([".gitignore", "version.dvc"]) From c6050efbe3ebde6d07ceb8c762df586ba7f1f670 Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Sun, 8 Dec 2019 14:35:51 +0530 Subject: [PATCH 06/14] codeclimate recommendation fix --- dvc/external_repo.py | 6 ++---- tests/func/test_external_repo.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index e4f6586b13..abe4c6d048 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -4,12 +4,10 @@ import tempfile from contextlib import contextmanager from distutils.dir_util import copy_tree - -from dvc.remote import RemoteConfig from funcy import retry - from dvc.exceptions import NoRemoteInExternalRepoError +from dvc.remote import RemoteConfig from dvc.config import NoRemoteError, ConfigError from dvc.exceptions import NoOutputInExternalRepoError from dvc.exceptions import OutputNotFoundError @@ -145,4 +143,4 @@ def _remote_config_exists(rconfig): default = rconfig.get_default() except ConfigError: default = None - return True if default else False + return bool(default) diff --git a/tests/func/test_external_repo.py b/tests/func/test_external_repo.py index d21e056a9d..e6a8ee7bce 100644 --- a/tests/func/test_external_repo.py +++ b/tests/func/test_external_repo.py @@ -1,8 +1,8 @@ import os -from dvc.repo import Repo from mock import patch +from dvc.repo import Repo from dvc.external_repo import external_repo from dvc.scm.git import Git from dvc.utils.fs import path_isin From 610e43a0ab95b87bb293834f12c920fb39de2a1e Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Sun, 8 Dec 2019 18:48:20 +0530 Subject: [PATCH 07/14] removed original repo ops --- dvc/external_repo.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index abe4c6d048..b85baa927a 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -71,18 +71,17 @@ def _external_repo(url=None, rev=None, cache_dir=None): try: # check if the URL is local and no default remote # add default remote pointing to the original repo's cache location - if not os.path.isdir(url): - # Adjust original repo for pointing remote towards its' cache - original_repo = Repo(url) - rconfig = RemoteConfig(original_repo.config) + if os.path.isdir(url): + rconfig = RemoteConfig(repo.config) if not _remote_config_exists(rconfig): rconfig.add( "upstream", - original_repo.cache.local.cache_dir, + repo.cache.local.cache_dir, default=True, + level=Config.LEVEL_LOCAL, ) - original_repo.scm.add([original_repo.config.config_file]) - original_repo.scm.commit("add remote") + repo.scm.add([repo.config.config_file]) + repo.scm.commit("add remote") if cache_dir is not None: cache_config = CacheConfig(repo.config) From 84616e8e5ed894faef00ef16f6bff4e1e38b3f4d Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Sun, 8 Dec 2019 20:25:24 +0530 Subject: [PATCH 08/14] removed commit logic --- dvc/external_repo.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 026c5bb13b..3f55f128ad 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -68,9 +68,8 @@ def _external_repo(url=None, rev=None, cache_dir=None): _git_checkout(new_path, rev) repo = Repo(new_path) - original_repo = None try: - # check if the URL is local and no default remote + # check if the URL is local and no default remote is present # add default remote pointing to the original repo's cache location if os.path.isdir(url): rconfig = RemoteConfig(repo.config) @@ -81,8 +80,6 @@ def _external_repo(url=None, rev=None, cache_dir=None): default=True, level=Config.LEVEL_LOCAL, ) - repo.scm.add([repo.config.config_file]) - repo.scm.commit("add remote") if cache_dir is not None: cache_config = CacheConfig(repo.config) @@ -90,8 +87,6 @@ def _external_repo(url=None, rev=None, cache_dir=None): finally: # Need to close/reopen repo to force config reread repo.close() - if original_repo: - original_repo.close() REPO_CACHE[key] = new_path return new_path From 5b9daea2b3885383d6332a7b676bfbd04626fc1a Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Mon, 9 Dec 2019 12:11:49 +0530 Subject: [PATCH 09/14] fixed test case --- dvc/external_repo.py | 4 +++- tests/func/test_external_repo.py | 9 ++++++--- tests/func/test_get.py | 1 - 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 3f55f128ad..ed4b8b39e6 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -74,12 +74,14 @@ def _external_repo(url=None, rev=None, cache_dir=None): if os.path.isdir(url): rconfig = RemoteConfig(repo.config) if not _remote_config_exists(rconfig): + original_repo = Repo(url) rconfig.add( "upstream", - repo.cache.local.cache_dir, + original_repo.cache.local.cache_dir, default=True, level=Config.LEVEL_LOCAL, ) + original_repo.close() if cache_dir is not None: cache_config = CacheConfig(repo.config) diff --git a/tests/func/test_external_repo.py b/tests/func/test_external_repo.py index e6a8ee7bce..4c34505a02 100644 --- a/tests/func/test_external_repo.py +++ b/tests/func/test_external_repo.py @@ -1,3 +1,4 @@ +import filecmp import os from mock import patch @@ -30,8 +31,10 @@ def test_external_repo(erepo): def test_external_repo_import_without_remote(erepo, dvc_repo): - src = erepo.CODE - dst = dvc_repo.root_dir + src = erepo.FOO + dst = os.path.join(dvc_repo.root_dir, "foo") Repo.get(erepo.root_dir, src, dst) - assert os.path.exists(dst + "/" + erepo.CODE) + assert os.path.exists(dst) + assert os.path.isfile(dst) + assert filecmp.cmp(erepo.FOO, dst, shallow=False) diff --git a/tests/func/test_get.py b/tests/func/test_get.py index 07551c5f36..8b99547f03 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -101,7 +101,6 @@ def test_get_from_non_dvc_master(erepo, tmp_path, monkeypatch, caplog): with caplog.at_level(logging.INFO, logger="dvc"): Repo.get(erepo._root_dir, erepo.FOO, out=imported_file, rev="branch") - assert caplog.text == "" assert filecmp.cmp( os.path.join(erepo._root_dir, erepo.FOO), imported_file, shallow=False ) From b27136884551a7dd86a9173764423fddfd122ab2 Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Mon, 9 Dec 2019 14:14:49 +0530 Subject: [PATCH 10/14] Moved `remote_config_exists` into `RemoteConfig` --- dvc/external_repo.py | 20 ++------------------ dvc/remote/config.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index ed4b8b39e6..4e9c50e981 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -8,7 +8,7 @@ from dvc.exceptions import NoRemoteInExternalRepoError from dvc.remote import RemoteConfig -from dvc.config import NoRemoteError, ConfigError +from dvc.config import NoRemoteError from dvc.exceptions import NoOutputInExternalRepoError from dvc.exceptions import OutputNotFoundError from dvc.utils.fs import remove @@ -73,7 +73,7 @@ def _external_repo(url=None, rev=None, cache_dir=None): # add default remote pointing to the original repo's cache location if os.path.isdir(url): rconfig = RemoteConfig(repo.config) - if not _remote_config_exists(rconfig): + if not rconfig.remote_config_exists(): original_repo = Repo(url) rconfig.add( "upstream", @@ -127,19 +127,3 @@ def _clone_repo(url, path): git = Git.clone(url, path) git.close() - - -def _remote_config_exists(rconfig): - """ - Checks if default remote config is present. - Args: - rconfig: a remote config - - Returns: - True if the remote config exists, else False - """ - try: - default = rconfig.get_default() - except ConfigError: - default = None - return bool(default) diff --git a/dvc/remote/config.py b/dvc/remote/config.py index dccad52ba3..e1dc2dfb94 100644 --- a/dvc/remote/config.py +++ b/dvc/remote/config.py @@ -156,3 +156,18 @@ def get_default(self, level=None): return self.config.get( Config.SECTION_CORE, Config.SECTION_CORE_REMOTE, level=level ) + + def remote_config_exists(self): + """ + Checks if default remote config is present. + Args: + rconfig: a remote config + + Returns: + True if the remote config exists, else False + """ + try: + default = self.get_default() + except ConfigError: + default = None + return bool(default) From a1ae3578c081db079bf0f715e5b70c10180e8b36 Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Tue, 10 Dec 2019 11:49:19 +0530 Subject: [PATCH 11/14] Fix caplog test case and move call to CmdRemoteAdd --- dvc/command/remote.py | 4 ++++ dvc/external_repo.py | 18 ++++++++++-------- dvc/remote/config.py | 9 ++++----- tests/func/test_external_repo.py | 12 ------------ tests/func/test_get.py | 1 + 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/dvc/command/remote.py b/dvc/command/remote.py index 8c6dbc3ef3..683c040f42 100644 --- a/dvc/command/remote.py +++ b/dvc/command/remote.py @@ -19,6 +19,10 @@ def __init__(self, args): class CmdRemoteAdd(CmdRemoteConfig): def run(self): + if self.args.default: + logger.info( + "Setting '{}' as a default remote.".format(self.args.name) + ) self.remote_config.add( self.args.name, self.args.url, diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 4e9c50e981..9aca3c2f93 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -73,15 +73,17 @@ def _external_repo(url=None, rev=None, cache_dir=None): # add default remote pointing to the original repo's cache location if os.path.isdir(url): rconfig = RemoteConfig(repo.config) - if not rconfig.remote_config_exists(): + if not rconfig.remote_remote_set(): original_repo = Repo(url) - rconfig.add( - "upstream", - original_repo.cache.local.cache_dir, - default=True, - level=Config.LEVEL_LOCAL, - ) - original_repo.close() + try: + rconfig.add( + "aut-generated-upstream", + original_repo.cache.local.cache_dir, + default=True, + level=Config.LEVEL_LOCAL, + ) + finally: + original_repo.close() if cache_dir is not None: cache_config = CacheConfig(repo.config) diff --git a/dvc/remote/config.py b/dvc/remote/config.py index e1dc2dfb94..b4d12baaab 100644 --- a/dvc/remote/config.py +++ b/dvc/remote/config.py @@ -105,7 +105,6 @@ def add(self, name, url, default=False, force=False, level=None): level=level, ) if default: - logger.info("Setting '{}' as a default remote.".format(name)) self.config.set( Config.SECTION_CORE, Config.SECTION_CORE_REMOTE, @@ -157,7 +156,7 @@ def get_default(self, level=None): Config.SECTION_CORE, Config.SECTION_CORE_REMOTE, level=level ) - def remote_config_exists(self): + def remote_remote_set(self): """ Checks if default remote config is present. Args: @@ -167,7 +166,7 @@ def remote_config_exists(self): True if the remote config exists, else False """ try: - default = self.get_default() + self.get_default() + return True except ConfigError: - default = None - return bool(default) + return False diff --git a/tests/func/test_external_repo.py b/tests/func/test_external_repo.py index 4c34505a02..de2b8b20bb 100644 --- a/tests/func/test_external_repo.py +++ b/tests/func/test_external_repo.py @@ -1,9 +1,7 @@ -import filecmp import os from mock import patch -from dvc.repo import Repo from dvc.external_repo import external_repo from dvc.scm.git import Git from dvc.utils.fs import path_isin @@ -28,13 +26,3 @@ def test_external_repo(erepo): assert path_isin(repo.cache.local.cache_dir, repo.root_dir) assert mock.call_count == 1 - - -def test_external_repo_import_without_remote(erepo, dvc_repo): - src = erepo.FOO - dst = os.path.join(dvc_repo.root_dir, "foo") - - Repo.get(erepo.root_dir, src, dst) - assert os.path.exists(dst) - assert os.path.isfile(dst) - assert filecmp.cmp(erepo.FOO, dst, shallow=False) diff --git a/tests/func/test_get.py b/tests/func/test_get.py index 8b99547f03..07551c5f36 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -101,6 +101,7 @@ def test_get_from_non_dvc_master(erepo, tmp_path, monkeypatch, caplog): with caplog.at_level(logging.INFO, logger="dvc"): Repo.get(erepo._root_dir, erepo.FOO, out=imported_file, rev="branch") + assert caplog.text == "" assert filecmp.cmp( os.path.join(erepo._root_dir, erepo.FOO), imported_file, shallow=False ) From 40c392d3123c5a00d67f406de8de41df7d7cf418 Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Tue, 10 Dec 2019 15:31:58 +0530 Subject: [PATCH 12/14] type and doc fix --- dvc/external_repo.py | 2 +- dvc/remote/config.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index 9aca3c2f93..fe5792f133 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -73,7 +73,7 @@ def _external_repo(url=None, rev=None, cache_dir=None): # add default remote pointing to the original repo's cache location if os.path.isdir(url): rconfig = RemoteConfig(repo.config) - if not rconfig.remote_remote_set(): + if not rconfig.default_remote_set(): original_repo = Repo(url) try: rconfig.add( diff --git a/dvc/remote/config.py b/dvc/remote/config.py index b4d12baaab..53735f5c11 100644 --- a/dvc/remote/config.py +++ b/dvc/remote/config.py @@ -156,14 +156,14 @@ def get_default(self, level=None): Config.SECTION_CORE, Config.SECTION_CORE_REMOTE, level=level ) - def remote_remote_set(self): + def default_remote_set(self): """ Checks if default remote config is present. Args: rconfig: a remote config Returns: - True if the remote config exists, else False + True if the remote config is set, else False """ try: self.get_default() From e6285f70c57017d05ca160015093a72e48756d9f Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Tue, 10 Dec 2019 15:38:54 +0530 Subject: [PATCH 13/14] typo in doc fix --- dvc/remote/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/remote/config.py b/dvc/remote/config.py index 53735f5c11..2e9e204469 100644 --- a/dvc/remote/config.py +++ b/dvc/remote/config.py @@ -163,7 +163,7 @@ def default_remote_set(self): rconfig: a remote config Returns: - True if the remote config is set, else False + True if the default remote config is set, else False """ try: self.get_default() From e1e0cbd0383c0a8227f82447b29cde54c81fa2e9 Mon Sep 17 00:00:00 2001 From: Mayur Kularni Date: Wed, 11 Dec 2019 10:54:59 +0530 Subject: [PATCH 14/14] moved default check out of `RemoteConfig` --- dvc/external_repo.py | 23 ++++++++++++++++++++--- dvc/remote/config.py | 15 --------------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index fe5792f133..758d8f4215 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -4,11 +4,12 @@ import tempfile from contextlib import contextmanager from distutils.dir_util import copy_tree + from funcy import retry +from dvc.config import NoRemoteError, ConfigError from dvc.exceptions import NoRemoteInExternalRepoError from dvc.remote import RemoteConfig -from dvc.config import NoRemoteError from dvc.exceptions import NoOutputInExternalRepoError from dvc.exceptions import OutputNotFoundError from dvc.utils.fs import remove @@ -73,11 +74,11 @@ def _external_repo(url=None, rev=None, cache_dir=None): # add default remote pointing to the original repo's cache location if os.path.isdir(url): rconfig = RemoteConfig(repo.config) - if not rconfig.default_remote_set(): + if not _default_remote_set(rconfig): original_repo = Repo(url) try: rconfig.add( - "aut-generated-upstream", + "auto-generated-upstream", original_repo.cache.local.cache_dir, default=True, level=Config.LEVEL_LOCAL, @@ -129,3 +130,19 @@ def _clone_repo(url, path): git = Git.clone(url, path) git.close() + + +def _default_remote_set(rconfig): + """ + Checks if default remote config is present. + Args: + rconfig: a remote config + + Returns: + True if the default remote config is set, else False + """ + try: + rconfig.get_default() + return True + except ConfigError: + return False diff --git a/dvc/remote/config.py b/dvc/remote/config.py index 2e9e204469..bbda7fc445 100644 --- a/dvc/remote/config.py +++ b/dvc/remote/config.py @@ -155,18 +155,3 @@ def get_default(self, level=None): return self.config.get( Config.SECTION_CORE, Config.SECTION_CORE_REMOTE, level=level ) - - def default_remote_set(self): - """ - Checks if default remote config is present. - Args: - rconfig: a remote config - - Returns: - True if the default remote config is set, else False - """ - try: - self.get_default() - return True - except ConfigError: - return False