From 6fcafc912157485f40530fa6c4003c7ff83aceea Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 29 Dec 2020 14:32:29 +0200 Subject: [PATCH] erepo: make repo configuration explicit (#5174) --- dvc/config.py | 9 ++- dvc/external_repo.py | 110 ++++++++++++++----------------- dvc/repo/__init__.py | 3 +- tests/unit/test_external_repo.py | 4 +- 4 files changed, 59 insertions(+), 67 deletions(-) diff --git a/dvc/config.py b/dvc/config.py index f7e7f60cd..f5d453621 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -266,7 +266,7 @@ class Config(dict): CONFIG_LOCAL = "config.local" def __init__( - self, dvc_dir=None, validate=True, tree=None, + self, dvc_dir=None, validate=True, tree=None, config=None, ): # pylint: disable=super-init-not-called from dvc.tree.local import LocalTree @@ -285,7 +285,7 @@ class Config(dict): self.wtree = LocalTree(None, {"url": self.dvc_dir}) self.tree = tree or self.wtree - self.load(validate=validate) + self.load(validate=validate, config=config) @classmethod def get_dir(cls, level): @@ -325,7 +325,7 @@ class Config(dict): open(config_file, "w+").close() return Config(dvc_dir) - def load(self, validate=True): + def load(self, validate=True, config=None): """Loads config from all the config files. Raises: @@ -333,6 +333,9 @@ class Config(dict): """ conf = self.load_config_to_level() + if config is not None: + merge(conf, config) + if validate: conf = self.validate(conf) diff --git a/dvc/external_repo.py b/dvc/external_repo.py index ecf9fe009..fd53c1f87 100644 --- a/dvc/external_repo.py +++ b/dvc/external_repo.py @@ -8,7 +8,6 @@ from typing import Dict, Iterable from funcy import cached_property, reraise, retry, wrap_with -from dvc.cache import Cache from dvc.config import NoRemoteError, NotDvcRepoError from dvc.exceptions import ( DvcException, @@ -87,6 +86,30 @@ def clean_repos(): _remove(path) +def _get_remote_config(url): + try: + repo = Repo(url) + except NotDvcRepoError: + return {} + + try: + name = repo.config["core"].get("remote") + if not name: + # Fill the empty upstream entry with a new remote pointing to the + # original repo's cache location. + name = "auto-generated-upstream" + return { + "core": {"remote": name}, + "remote": {name: {"url": repo.cache.local.cache_dir}}, + } + + # Use original remote to make sure that we are using correct url, + # credential paths, etc if they are relative to the config location. + return {"remote": {name: repo.config["remote"][name]}} + finally: + repo.close() + + class ExternalRepo(Repo): # pylint: disable=no-member @@ -102,19 +125,29 @@ class ExternalRepo(Repo): uninitialized=False, **kwargs, ): - super().__init__( - root_dir, scm=scm, rev=rev, uninitialized=uninitialized - ) - self.url = url self.for_write = for_write - self.cache_dir = cache_dir or self._get_cache_dir() - self.cache_types = cache_types - - self._setup_cache(self) - self._fix_upstream(self) self.tree_confs = kwargs + self._cache_config = { + "cache": { + "dir": cache_dir or self._get_cache_dir(), + "type": cache_types, + } + } + + config = self._cache_config.copy() + if os.path.isdir(url): + config.update(_get_remote_config(url)) + + super().__init__( + root_dir, + scm=scm, + rev=rev, + uninitialized=uninitialized, + config=config, + ) + def __str__(self): return self.url @@ -218,58 +251,13 @@ class ExternalRepo(Repo): kw["fetch"] = True return RepoTree(repo, **kw) - @staticmethod - def _fix_local_remote(orig_repo, src_repo, remote_name): - # If a remote URL is relative to the source repo, - # it will have changed upon config load and made - # relative to this new repo. Restore the old one here. - new_remote = orig_repo.config["remote"][remote_name] - old_remote = src_repo.config["remote"][remote_name] - if new_remote["url"] != old_remote["url"]: - new_remote["url"] = old_remote["url"] - - @staticmethod - def _add_upstream(orig_repo, src_repo): - # Fill the empty upstream entry with a new remote pointing to the - # original repo's cache location. - cache_dir = src_repo.cache.local.cache_dir - orig_repo.config["remote"]["auto-generated-upstream"] = { - "url": cache_dir - } - orig_repo.config["core"]["remote"] = "auto-generated-upstream" - def make_repo(self, path): - repo = Repo(path, scm=self.scm, rev=self.get_rev()) - - self._setup_cache(repo) - self._fix_upstream(repo) - - return repo - - def _setup_cache(self, repo): - repo.config["cache"]["dir"] = self.cache_dir - repo.cache = Cache(repo) - if self.cache_types: - repo.cache.local.cache_types = self.cache_types - - def _fix_upstream(self, repo): - if not os.path.isdir(self.url): - return - - try: - rel_path = os.path.relpath(repo.root_dir, self.root_dir) - src_repo = Repo(PathInfo(self.url) / rel_path) - except NotDvcRepoError: - return - - try: - remote_name = repo.config["core"].get("remote") - if remote_name: - self._fix_local_remote(repo, src_repo, remote_name) - else: - self._add_upstream(repo, src_repo) - finally: - src_repo.close() + config = self._cache_config.copy() + if os.path.isdir(self.url): + rel = os.path.relpath(path, self.root_dir) + repo_path = os.path.join(self.url, rel) + config.update(_get_remote_config(repo_path)) + return Repo(path, scm=self.scm, rev=self.get_rev(), config=config) @wrap_with(threading.Lock()) def _get_cache_dir(self): diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index e2b9b2170..fcc76bff4 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -128,6 +128,7 @@ class Repo: rev=None, subrepos=False, uninitialized=False, + config=None, ): from dvc.cache import Cache from dvc.data_cloud import DataCloud @@ -150,7 +151,7 @@ class Repo: else: self.tree = LocalTree(self, {"url": self.root_dir}, **tree_kwargs) - self.config = Config(self.dvc_dir, tree=self.tree) + self.config = Config(self.dvc_dir, tree=self.tree, config=config) self._scm = scm # used by RepoTree to determine if it should traverse subrepos diff --git a/tests/unit/test_external_repo.py b/tests/unit/test_external_repo.py index 97684f834..1b501830e 100644 --- a/tests/unit/test_external_repo.py +++ b/tests/unit/test_external_repo.py @@ -60,11 +60,11 @@ def test_subrepo_is_constructed_properly( subrepo = spy.return_value assert repo.url == str(tmp_dir) - assert repo.cache_dir == str(cache_dir) + assert repo._cache_config["cache"]["dir"] == str(cache_dir) assert repo.cache.local.cache_dir == str(cache_dir) assert subrepo.cache.local.cache_dir == str(cache_dir) - assert repo.cache_types == ["symlink"] + assert repo._cache_config["cache"]["type"] == ["symlink"] assert repo.cache.local.cache_types == ["symlink"] assert subrepo.cache.local.cache_types == ["symlink"]