Stone_Red/dvc
1
0
Fork 0
mirror of https://github.com/Stone-Red-Code/dvc.git synced 2026-11-02 07:18:25 +01:00
Code Issues Packages Projects Releases Wiki Activity

dvc: drop cached external outputs

Browse Source 
Fixes #9531

Docs https://github.com/iterative/dvc.org/pull/4574 Kudos @dberenbaum
This commit is contained in:
Ruslan Kuprieiev
2023-06-09 18:34:36 +03:00
parent 8b1c1a31f3
commit af1fb722c8
20 changed files with 69 additions and 311 deletions
Download Patch File Download Diff File Expand all files Collapse all files
dvc/cachemgr.py
-11
View File
@@ -32,13 +32,6 @@ def _get_odb(
class CacheManager:
CACHE_DIR = "cache"
CLOUD_SCHEMES = [
Schemes.S3,
Schemes.GS,
Schemes.SSH,
Schemes.HDFS,
Schemes.WEBHDFS,
]
FILES_DIR = "files"
def __init__(self, repo):
@@ -91,16 +84,12 @@ class CacheManager:
)
def __getattr__(self, name):
if name not in self._odb and name in self.CLOUD_SCHEMES:
self._init_odb([name])
try:
return self._odb[name]
except KeyError as exc:
raise AttributeError from exc
def by_scheme(self):
self._init_odb(self.CLOUD_SCHEMES)
yield from self._odb.items()
@property
dvc/commands/add.py
-9
View File
@@ -24,8 +24,6 @@ class CmdAdd(CmdBase):
invalid_opt = "--glob option"
elif args.no_commit:
invalid_opt = "--no-commit option"
elif args.external:
invalid_opt = "--external option"
else:
message = "{option} can't be used without --to-remote"
if args.remote:
@@ -49,7 +47,6 @@ class CmdAdd(CmdBase):
self.repo.add(
self.args.targets,
no_commit=self.args.no_commit,
external=self.args.external,
glob=self.args.glob,
out=self.args.out,
remote=self.args.remote,
@@ -82,12 +79,6 @@ def add_parser(subparsers, parent_parser):
default=False,
help="Don't put files/directories into cache.",
)
parser.add_argument(
"--external",
action="store_true",
default=False,
help="Allow targets that are outside of the DVC repository.",
)
parser.add_argument(
"--glob",
action="store_true",
dvc/config_schema.py
+7 -7
View File
@@ -154,13 +154,13 @@ SCHEMA = {
"machine": Lower,
},
"cache": {
"local": str,
"s3": str,
"gs": str,
"hdfs": str,
"webhdfs": str,
"ssh": str,
"azure": str,
"local": str, # obsoleted
"s3": str, # obsoleted
"gs": str, # obsoleted
"hdfs": str, # obsoleted
"webhdfs": str, # obsoleted
"ssh": str, # obsoleted
"azure": str, # obsoleted
# This is for default local cache
"dir": str,
**LOCAL_COMMON,
dvc/exceptions.py
-15
View File
@@ -299,21 +299,6 @@ class URLMissingError(DvcException):
super().__init__(f"The path '{url}' does not exist")
class RemoteCacheRequiredError(DvcException):
def __init__(self, scheme, fs_path):
super().__init__(
(
"Current operation was unsuccessful because '{}' requires "
"existing cache on '{}' remote. See {} for information on how "
"to set up remote cache."
).format(
fs_path,
scheme,
format_link("https://man.dvc.org/config#cache"),
)
)
class IsADirectoryError(DvcException): # noqa,pylint:disable=redefined-builtin
"""Raised when a file operation is requested on a directory."""
dvc/output.py
+3 -9
View File
@@ -19,7 +19,6 @@ from dvc.exceptions import (
ConfirmRemoveError,
DvcException,
MergeError,
RemoteCacheRequiredError,
)
from dvc.utils.objects import cached_property
from dvc_data.hashfile import check as ocheck
@@ -526,14 +525,9 @@ class Output:
def cache(self):
from dvc.cachemgr import LEGACY_HASH_NAMES
if self.is_in_repo:
odb_name = "legacy" if self.hash_name in LEGACY_HASH_NAMES else "repo"
else:
odb_name = self.protocol
odb = getattr(self.repo.cache, odb_name)
if self.use_cache and odb is None:
raise RemoteCacheRequiredError(self.fs.protocol, self.fs_path)
return odb
assert self.is_in_repo
odb_name = "legacy" if self.hash_name in LEGACY_HASH_NAMES else "repo"
return getattr(self.repo.cache, odb_name)
@property
def local_cache(self):
dvc/repo/add.py
-4
View File
@@ -54,7 +54,6 @@ PIPELINE_TRACKED_UPDATE_FMT = (
def get_or_create_stage(
repo: "Repo",
target: str,
external: bool = False,
out: Optional[str] = None,
to_remote: bool = False,
force: bool = False,
@@ -79,7 +78,6 @@ def get_or_create_stage(
fname=path,
wdir=wdir,
outs=[out],
external=external,
force=force,
)
return StageInfo(stage, output_exists=False)
@@ -199,7 +197,6 @@ def add(
repo: "Repo",
targets: Union["StrOrBytesPath", Iterator["StrOrBytesPath"]],
no_commit: bool = False,
external: bool = False,
glob: bool = False,
out: Optional[str] = None,
remote: Optional[str] = None,
@@ -215,7 +212,6 @@ def add(
target: get_or_create_stage(
repo,
target,
external=external,
out=out,
to_remote=to_remote,
force=force,
dvc/stage/__init__.py
+2 -3
View File
@@ -89,7 +89,7 @@ class RawData:
generated_from: Optional[str] = None
def create_stage(cls: Type[_T], repo, path, external=False, **kwargs) -> _T:
def create_stage(cls: Type[_T], repo, path, **kwargs) -> _T:
from dvc.dvcfile import check_dvcfile_path
wdir = os.path.abspath(kwargs.get("wdir", None) or os.curdir)
@@ -101,8 +101,7 @@ def create_stage(cls: Type[_T], repo, path, external=False, **kwargs) -> _T:
stage = loads_from(cls, repo, path, wdir, kwargs)
fill_stage_outputs(stage, **kwargs)
if not external:
check_no_externals(stage)
check_no_externals(stage)
fill_stage_dependencies(
stage, **project(kwargs, ["deps", "erepo", "params", "fs_config"])
)
dvc/stage/cache.py
+7 -3
View File
@@ -44,7 +44,12 @@ def _can_hash(stage):
return False
for out in stage.outs:
if out.protocol != "local" or not out.def_path or out.persist:
if (
out.protocol != "local"
or not out.def_path
or out.persist
or not out.is_in_repo
):
return False
return True
@@ -112,7 +117,6 @@ class StageCache:
cmd=cache["cmd"],
wdir=wdir,
outs=[out["path"] for out in cache["outs"]],
external=True,
)
StageLoader.fill_from_lock(stage, cache)
return stage
@@ -137,7 +141,7 @@ class StageCache:
# NOTE: using copy link to make it look like a git-tracked file
with self._cache_type_copy():
for out in cached_stage.outs:
if out.def_path in outs_no_cache:
if out.def_path in outs_no_cache and out.is_in_repo:
yield out
def save(self, stage):
dvc/stage/utils.py
+8 -12
View File
@@ -87,28 +87,24 @@ def fill_stage_dependencies(stage, deps=None, erepo=None, params=None, fs_config
def check_no_externals(stage):
from urllib.parse import urlparse
from dvc.utils import format_link
# NOTE: preventing users from accidentally using external outputs. See
# https://github.com/iterative/dvc/issues/1545 for more details.
def _is_external(out):
# NOTE: in case of `remote://` notation, the user clearly knows that
# this is an advanced feature and so we shouldn't error-out.
if out.is_in_repo or urlparse(out.def_path).scheme == "remote":
def _is_cached_external(out):
if out.is_in_repo or not out.use_cache:
return False
return True
outs = [str(out) for out in stage.outs if _is_external(out)]
outs = [str(out) for out in stage.outs if _is_cached_external(out)]
if not outs:
return
str_outs = ", ".join(outs)
link = format_link("https://dvc.org/doc/user-guide/managing-external-data")
link = format_link(
"https://dvc.org/doc/user-guide/pipelines/external-dependencies-and-outputs"
)
raise StageExternalOutputsError(
f"Output(s) outside of DVC project: {str_outs}. See {link} for more info."
f"Cached output(s) outside of DVC project: {str_outs}. "
f"See {link} for more info."
)
dvc/testing/workspace_tests.py
-51
View File
@@ -153,57 +153,6 @@ class TestImportURLVersionAware:
assert dvc.status() == {}
class TestAdd:
@pytest.fixture
def hash_name(self):
pytest.skip()
@pytest.fixture
def hash_value(self):
pytest.skip()
@pytest.fixture
def dir_hash_value(self):
pytest.skip()
def test_add(self, tmp_dir, dvc, workspace, hash_name, hash_value):
from dvc.stage.exceptions import StageExternalOutputsError
workspace.gen("file", "file")
with pytest.raises(StageExternalOutputsError):
dvc.add(workspace.url)
dvc.add("remote://workspace/file")
assert (tmp_dir / "file.dvc").read_text() == (
"outs:\n"
f"- {hash_name}: {hash_value}\n"
" size: 4\n"
" hash: md5\n"
" path: remote://workspace/file\n"
)
assert (workspace / "file").read_text() == "file"
assert (
workspace / "cache" / "files" / "md5" / hash_value[:2] / hash_value[2:]
).read_text() == "file"
assert dvc.status() == {}
# pylint: disable-next=unused-argument
def test_add_dir(self, tmp_dir, dvc, workspace, hash_name, dir_hash_value):
workspace.gen({"dir": {"file": "file", "subdir": {"subfile": "subfile"}}})
dvc.add("remote://workspace/dir")
assert (
workspace
/ "cache"
/ "files"
/ "md5"
/ dir_hash_value[:2]
/ dir_hash_value[2:]
).is_file()
def match_files(fs, entries, expected):
entries_content = {(fs.path.normpath(d["path"]), d["isdir"]) for d in entries}
expected_content = {(fs.path.normpath(d["path"]), d["isdir"]) for d in expected}
tests/func/test_add.py
+6 -68
View File
@@ -22,7 +22,6 @@ from dvc.output import (
)
from dvc.stage import Stage
from dvc.stage.exceptions import StageExternalOutputsError, StagePathNotFoundError
from dvc.testing.workspace_tests import TestAdd
from dvc.utils.fs import path_isin
from dvc.utils.serialize import YAMLFileCorruptedError
from dvc_data.hashfile.hash import file_md5
@@ -234,55 +233,6 @@ def test_add_filtered_files_in_dir(
assert stage.outs[0].def_path in expected_def_paths
class TestAddExternal(TestAdd):
@pytest.fixture
def hash_name(self):
return "md5"
@pytest.fixture
def hash_value(self):
return "8c7dd922ad47494fc02c388e12c00eac"
@pytest.fixture
def dir_hash_value(self):
return "b6dcab6ccd17ca0a8bf4a215a37d14cc.dir"
def test_add_external_relpath(tmp_dir, dvc, local_cloud):
(fpath,) = local_cloud.gen("file", "file")
rel = os.path.relpath(fpath)
with pytest.raises(StageExternalOutputsError):
dvc.add(rel)
dvc.add(rel, external=True)
assert (tmp_dir / "file.dvc").read_text() == (
"outs:\n"
"- md5: 8c7dd922ad47494fc02c388e12c00eac\n"
" size: 4\n"
" hash: md5\n"
f" path: {rel}\n"
)
assert fpath.read_text() == "file"
assert dvc.status() == {}
def test_add_local_remote_file(tmp_dir, dvc):
"""
Making sure that 'remote' syntax is handled properly for local outs.
"""
tmp_dir.gen({"foo": "foo", "bar": "bar"})
tmp_dir.add_remote(url=tmp_dir.fs_path, name="myremote")
assert main(["add", "remote://myremote/foo"]) == 0
d = (tmp_dir / "foo.dvc").load_yaml()
assert d["outs"][0]["path"] == "remote://myremote/foo"
assert main(["add", (tmp_dir / "bar").fs_path]) == 0
d = (tmp_dir / "bar.dvc").load_yaml()
assert d["outs"][0]["path"] == "bar"
def test_cmd_add(tmp_dir, dvc):
tmp_dir.gen("foo", "foo")
ret = main(["add", "foo"])
@@ -755,15 +705,9 @@ def test_add_symlink_file(tmp_dir, dvc):
dvc.add(os.path.join("dir", "foo"))
@pytest.mark.parametrize("external", [True, False])
def test_add_symlink_dir(make_tmp_dir, tmp_dir, dvc, external):
if external:
data_dir = make_tmp_dir("data")
data_dir.gen({"foo": "foo"})
target = os.fspath(data_dir)
else:
tmp_dir.gen({"data": {"foo": "foo"}})
target = os.path.join(".", "data")
def test_add_symlink_dir(make_tmp_dir, tmp_dir, dvc):
tmp_dir.gen({"data": {"foo": "foo"}})
target = os.path.join(".", "data")
tmp_dir.gen({"data": {"foo": "foo"}})
@@ -774,15 +718,9 @@ def test_add_symlink_dir(make_tmp_dir, tmp_dir, dvc, external):
dvc.add("dir")
@pytest.mark.parametrize("external", [True, False])
def test_add_file_in_symlink_dir(make_tmp_dir, tmp_dir, dvc, external):
if external:
data_dir = make_tmp_dir("data")
data_dir.gen({"dir": {"foo": "foo"}})
target = os.fspath(data_dir / "dir")
else:
tmp_dir.gen({"data": {"foo": "foo"}})
target = os.path.join(".", "data")
def test_add_file_in_symlink_dir(make_tmp_dir, tmp_dir, dvc):
tmp_dir.gen({"data": {"foo": "foo"}})
target = os.path.join(".", "data")
(tmp_dir / "dir").symlink_to(target)
tests/func/test_gc.py
-28
View File
@@ -251,34 +251,6 @@ def test_gc_not_collect_pipeline_tracked_files(tmp_dir, dvc, run_copy):
assert _count_files(dvc.cache.local.path) == 0
def test_gc_external_output(tmp_dir, dvc, workspace):
workspace.gen({"foo": "foo", "bar": "bar"})
(foo_stage,) = dvc.add("remote://workspace/foo")
(bar_stage,) = dvc.add("remote://workspace/bar")
foo_hash = foo_stage.outs[0].hash_info.value
bar_hash = bar_stage.outs[0].hash_info.value
assert (
workspace / "cache" / "files" / "md5" / foo_hash[:2] / foo_hash[2:]
).read_text() == "foo"
assert (
workspace / "cache" / "files" / "md5" / bar_hash[:2] / bar_hash[2:]
).read_text() == "bar"
(tmp_dir / "foo.dvc").unlink()
dvc.gc(workspace=True)
assert not (
workspace / "cache" / "files" / "md5" / foo_hash[:2] / foo_hash[2:]
).exists()
assert (
workspace / "cache" / "files" / "md5" / bar_hash[:2] / bar_hash[2:]
).read_text() == "bar"
def test_gc_all_experiments(tmp_dir, scm, dvc):
from dvc.repo.experiments.refs import ExpRefInfo
tests/func/test_get.py
-14
View File
@@ -129,20 +129,6 @@ def test_get_a_dvc_file(tmp_dir, erepo_dir):
Repo.get(os.fspath(erepo_dir), "some_file.dvc")
# https://github.com/iterative/dvc/pull/2837#discussion_r352123053
def test_get_full_dvc_path(tmp_dir, erepo_dir, tmp_path_factory):
path = tmp_path_factory.mktemp("ext")
external_data = path / "ext_data"
external_data.write_text("ext_data")
with erepo_dir.chdir():
erepo_dir.dvc.add(os.fspath(external_data), external=True)
erepo_dir.scm_add("ext_data.dvc", commit="add external data")
Repo.get(os.fspath(erepo_dir), os.fspath(external_data), "ext_data_imported")
assert (tmp_dir / "ext_data_imported").read_text() == "ext_data"
def test_non_cached_output(tmp_dir, erepo_dir):
src = "non_cached_file"
dst = src + "_imported"
tests/func/test_odb.py
-11
View File
@@ -81,17 +81,6 @@ def test_external_cache_dir(tmp_dir, dvc, make_tmp_dir):
assert len(os.listdir(cache_dir)) != 0
def test_remote_cache_references(tmp_dir, dvc):
with dvc.config.edit() as conf:
conf["remote"]["storage"] = {"url": "ssh://user@localhost:23"}
conf["remote"]["cache"] = {"url": "remote://storage/tmp"}
conf["cache"]["ssh"] = "cache"
dvc.cache = CacheManager(dvc)
assert dvc.cache.ssh.path == "/tmp/files/md5"
def test_shared_cache_dir(tmp_dir):
cache_dir = os.path.abspath(os.path.join(os.curdir, "cache"))
for d in ["dir1", "dir2"]:
tests/func/test_remote.py
+1 -17
View File
@@ -9,7 +9,7 @@ import pytest
from dvc.cli import main
from dvc.config import Config
from dvc.exceptions import DownloadError, RemoteCacheRequiredError, UploadError
from dvc.exceptions import DownloadError, UploadError
from dvc.utils.fs import remove
@@ -257,22 +257,6 @@ def test_remote_modify_local_on_repo_config(tmp_dir, dvc):
}
def test_external_dir_resource_on_no_cache(tmp_dir, dvc, tmp_path_factory):
# https://github.com/iterative/dvc/issues/2647, is some situations
# (external dir dependency) cache is required to calculate dir md5
external_dir = tmp_path_factory.mktemp("external_dir")
file = external_dir / "file"
dvc.cache.local = None
with pytest.raises(RemoteCacheRequiredError):
dvc.run(
cmd=f"echo content > {file}",
outs=[os.fspath(file)],
name="echo",
external=True,
)
def test_push_order(tmp_dir, dvc, tmp_path_factory, mocker, local_remote):
from dvc_objects.fs import generic
tests/func/test_repo_index.py
+25 -34
View File
@@ -143,17 +143,7 @@ def test_skip_graph_checks(dvc, mocker):
assert mock_build_graph.called
def get_index(dvc, rev):
if rev:
brancher = dvc.brancher(revs=[rev])
if rev != "workspace":
assert next(brancher) == "workspace"
next(brancher)
return Index.from_repo(dvc)
@pytest.mark.parametrize("rev", ["workspace", "HEAD"])
def test_used_objs(tmp_dir, scm, dvc, run_copy, rev):
def test_used_objs(tmp_dir, scm, dvc, run_copy):
from dvc_data.hashfile.hash_info import HashInfo
dvc.scm_context.autostage = True
@@ -161,30 +151,31 @@ def test_used_objs(tmp_dir, scm, dvc, run_copy, rev):
run_copy("foo", "bar", name="copy-foo-bar")
scm.commit("commit")
index = get_index(dvc, rev)
for _ in dvc.brancher(revs=["HEAD"]):
expected_objs = [
HashInfo(
name="md5",
value="acbd18db4cc2f85cedef654fccc4a4d8",
obj_name="bar",
),
HashInfo(
name="md5",
value="8c7dd922ad47494fc02c388e12c00eac",
obj_name="dir/subdir/file",
),
HashInfo(
name="md5",
value="d28c9e28591aeb7e303dc6772ffa6f6b.dir",
obj_name="dir",
),
]
expected_objs = [
HashInfo(
name="md5",
value="acbd18db4cc2f85cedef654fccc4a4d8",
obj_name="bar",
),
HashInfo(
name="md5",
value="8c7dd922ad47494fc02c388e12c00eac",
obj_name="dir/subdir/file",
),
HashInfo(
name="md5",
value="d28c9e28591aeb7e303dc6772ffa6f6b.dir",
obj_name="dir",
),
]
assert index.used_objs() == {None: set(expected_objs)}
assert index.used_objs("dir") == {None: set(expected_objs[1:])}
assert index.used_objs(".", recursive=True) == {None: set(expected_objs)}
assert index.used_objs("copy-foo-bar", with_deps=True) == {None: {expected_objs[0]}}
assert dvc.index.used_objs() == {None: set(expected_objs)}
assert dvc.index.used_objs("dir") == {None: set(expected_objs[1:])}
assert dvc.index.used_objs(".", recursive=True) == {None: set(expected_objs)}
assert dvc.index.used_objs("copy-foo-bar", with_deps=True) == {
None: {expected_objs[0]}
}
def test_view_granular_dir(tmp_dir, scm, dvc, run_copy):
tests/func/test_run.py
+4 -5
View File
@@ -534,7 +534,7 @@ def test_run_external_outputs(
name="mystage",
cmd="mycmd",
deps=["remote://workspace/foo"],
outs=["remote://workspace/bar"],
outs_no_cache=["remote://workspace/bar"],
no_exec=True,
)
@@ -545,7 +545,8 @@ def test_run_external_outputs(
" deps:\n"
" - remote://workspace/foo\n"
" outs:\n"
" - remote://workspace/bar\n"
" - remote://workspace/bar:\n"
" cache: false\n"
)
assert (tmp_dir / "dvc.yaml").read_text() == dvc_yaml
@@ -574,9 +575,7 @@ def test_run_external_outputs(
assert (local_workspace / "foo").read_text() == "foo"
assert (local_workspace / "bar").read_text() == "bar"
assert (
local_workspace / "cache" / "files" / "md5" / bar_hash[:2] / bar_hash[2:]
).read_text() == "bar"
assert not (local_workspace / "cache").exists()
def test_rerun_callback(dvc):
tests/unit/command/test_add.py
-3
View File
@@ -9,7 +9,6 @@ def test_add(mocker, dvc):
[
"add",
"--no-commit",
"--external",
"--glob",
"data",
]
@@ -25,7 +24,6 @@ def test_add(mocker, dvc):
["data"],
no_commit=True,
glob=True,
external=True,
out=None,
remote=None,
to_remote=False,
@@ -57,7 +55,6 @@ def test_add_to_remote(mocker):
["s3://bucket/foo"],
no_commit=False,
glob=False,
external=False,
out="bar",
remote="remote",
to_remote=True,
tests/unit/stage/test_stage.py
+5 -2
View File
@@ -120,5 +120,8 @@ def test_external_outs(tmp_path_factory, dvc):
with dvc.config.edit() as conf:
conf["remote"]["myremote"] = {"url": os.fspath(tmp_path)}
create_stage(Stage, dvc, "path.dvc", outs=["remote://myremote/foo"])
create_stage(Stage, dvc, "path.dvc", outs=[os.fspath(foo)], external=True)
with pytest.raises(StageExternalOutputsError):
create_stage(Stage, dvc, "path.dvc", outs=["remote://myremote/foo"])
create_stage(Stage, dvc, "path.dvc", outs_no_cache=["remote://myremote/foo"])
create_stage(Stage, dvc, "path.dvc", outs_no_cache=[os.fspath(foo)])
tests/unit/test_info.py
+1 -5
View File
@@ -85,14 +85,10 @@ def test_info_in_broken_git_repo(tmp_dir, dvc, scm, caplog):
def test_caches(tmp_dir, dvc, caplog):
tmp_dir.add_remote(name="sshcache", url="ssh://example.com/path", default=False)
with tmp_dir.dvc.config.edit() as conf:
conf["cache"]["ssh"] = "sshcache"
dvc_info = get_dvc_info()
# Order of cache types is runtime dependent
assert re.search("Caches: (local, ssh|ssh, local)", dvc_info)
assert re.search("Caches: local", dvc_info)
def test_remotes_empty(tmp_dir, dvc, caplog):