From b286c311f451261b16b63b6e3f33f88d09240136 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20N=C3=BCesch?= Date: Sat, 18 Sep 2021 21:36:50 +0200 Subject: [PATCH] reincorporate PR 6111 into current master, re: #4420 (#6419) * rebased PR 6111 onto master * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * use make_tmp_dir fixture in tests Co-authored-by: U119511 Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alexander Meier Co-authored-by: meierale --- dvc/config_schema.py | 4 ++++ dvc/data_cloud.py | 2 +- dvc/repo/__init__.py | 38 ++++++++++++++++++++++++++++---- tests/func/test_state.py | 16 ++++++++++++++ tests/unit/remote/test_index.py | 2 +- tests/unit/remote/test_remote.py | 16 ++++++++++++++ 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/dvc/config_schema.py b/dvc/config_schema.py index cd445d3ff..c3aa29d32 100644 --- a/dvc/config_schema.py +++ b/dvc/config_schema.py @@ -223,9 +223,13 @@ SCHEMA = { ) }, "state": { + "dir": str, "row_limit": All(Coerce(int), Range(1)), # obsoleted "row_cleanup_quota": All(Coerce(int), Range(0, 100)), # obsoleted }, + "index": { + "dir": str, + }, "machine": { str: { "cloud": All(Lower, Choices("aws", "azure")), diff --git a/dvc/data_cloud.py b/dvc/data_cloud.py index c9c394f5c..1242a4680 100644 --- a/dvc/data_cloud.py +++ b/dvc/data_cloud.py @@ -59,7 +59,7 @@ class DataCloud: from dvc.objects.db import get_odb cls, config, path_info = get_cloud_fs(self.repo, name=name) - config["tmp_dir"] = self.repo.tmp_dir + config["tmp_dir"] = self.repo.index_db_dir return get_odb(cls(**config), path_info, **config) def push( diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 49c2afe16..95d579554 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -120,6 +120,34 @@ class Repo: return root_dir, dvc_dir, tmp_dir + def _get_database_dir(self, db_name): + # NOTE: by default, store SQLite-based remote indexes and state's + # `links` and `md5s` caches in the repository itself to avoid any + # possible state corruption in 'shared cache dir' scenario, but allow + # user to override this through config when, say, the repository is + # located on a mounted volume — see + # https://github.com/iterative/dvc/issues/4420 + base_db_dir = self.config.get(db_name, {}).get("dir", None) + if not base_db_dir: + return self.tmp_dir + + import hashlib + + from dvc.utils.fs import makedirs + + root_dir_hash = hashlib.sha224( + self.root_dir.encode("utf-8") + ).hexdigest() + + db_dir = os.path.join( + base_db_dir, + self.DVC_DIR, + f"{os.path.basename(self.root_dir)}-{root_dir_hash[0:7]}", + ) + + makedirs(db_dir, exist_ok=True) + return db_dir + def __init__( self, root_dir=None, @@ -183,10 +211,8 @@ class Repo: friendly=True, ) - # NOTE: storing state and link_state in the repository itself to - # avoid any possible state corruption in 'shared cache dir' - # scenario. - self.state = State(self.root_dir, self.tmp_dir, self.dvcignore) + state_db_dir = self._get_database_dir("state") + self.state = State(self.root_dir, state_db_dir, self.dvcignore) self.odb = ODBManager(self) self.stage_cache = StageCache(self) @@ -452,6 +478,10 @@ class Repo: return RepoFileSystem(self, subrepos=self.subrepos, **self._fs_conf) + @cached_property + def index_db_dir(self): + return self._get_database_dir("index") + @contextmanager def open_by_relpath(self, path, remote=None, mode="r", encoding=None): """Opens a specified resource as a file descriptor""" diff --git a/tests/func/test_state.py b/tests/func/test_state.py index ed2fa77b6..f69afbad6 100644 --- a/tests/func/test_state.py +++ b/tests/func/test_state.py @@ -1,7 +1,9 @@ import os +import re from dvc.hash_info import HashInfo from dvc.path_info import PathInfo +from dvc.repo import Repo from dvc.state import State from dvc.utils import file_md5 @@ -77,3 +79,17 @@ def test_get_unused_links(tmp_dir, dvc): ) == {"bar"} ) + + +def test_state_dir_config(make_tmp_dir, dvc): + assert dvc.state.tmp_dir == dvc.tmp_dir + + index_dir = str(make_tmp_dir("tmp_index")) + repo = Repo(config={"state": {"dir": index_dir}}) + assert os.path.dirname(repo.state.tmp_dir) == os.path.join( + index_dir, ".dvc" + ) + assert re.match( + r"^test_state_dir_config0-([0-9a-f]+)$", + os.path.basename(repo.state.tmp_dir), + ) diff --git a/tests/unit/remote/test_index.py b/tests/unit/remote/test_index.py index 0c32f2e41..8195daf3d 100644 --- a/tests/unit/remote/test_index.py +++ b/tests/unit/remote/test_index.py @@ -8,7 +8,7 @@ from dvc.objects.db.index import ObjectDBIndex @pytest.fixture def index(dvc): - index_ = ObjectDBIndex(dvc.tmp_dir, "foo") + index_ = ObjectDBIndex(dvc.index_db_dir, "foo") yield index_ diff --git a/tests/unit/remote/test_remote.py b/tests/unit/remote/test_remote.py index 5106761a1..c212ab031 100644 --- a/tests/unit/remote/test_remote.py +++ b/tests/unit/remote/test_remote.py @@ -1,8 +1,11 @@ +import os + import pytest from dvc.fs import get_cloud_fs from dvc.fs.gs import GSFileSystem from dvc.fs.s3 import S3FileSystem +from dvc.objects.db import get_index def test_remote_with_hash_jobs(dvc): @@ -55,3 +58,16 @@ def test_makedirs_not_create_for_top_level_path(fs_cls, dvc, mocker): fs.makedirs(fs.PATH_CLS(url)) assert not mocked_client.called + + +def test_remote_index_dir_config(make_tmp_dir, dvc): + index_dir = str(make_tmp_dir("tmp_index")) + with dvc.config.edit() as conf: + conf["index"]["dir"] = index_dir + conf["remote"]["s3"] = {"url": "s3://bucket/name"} + + dvc.root_dir = "/usr/local/test_repo" + + assert os.path.dirname( + get_index(dvc.cloud.get_remote_odb(name="s3")).index_dir + ) == os.path.join(index_dir, ".dvc", "test_repo-a473718", "index")