deps/outs: get rid of self.info (#4502)

By itself `self.info` is quite confusing, as it is not clear what it is
about. Using `hash_info` makes much more sense and is required to
support alternative hash types.

Related to #4144, #3069, #1676
This commit is contained in:
Ruslan Kuprieiev
2020-08-31 22:46:18 +03:00
committed by GitHub
parent 4f8432886d
commit 56500601bd
16 changed files with 134 additions and 129 deletions
+4 -5
View File
@@ -620,17 +620,16 @@ class CloudCache:
assert their_info
if ancestor_info:
ancestor_hash = ancestor_info[self.tree.PARAM_CHECKSUM]
ancestor_hash = ancestor_info.value
ancestor = self.get_dir_cache(ancestor_hash)
else:
ancestor = []
our_hash = our_info[self.tree.PARAM_CHECKSUM]
our_hash = our_info.value
our = self.get_dir_cache(our_hash)
their_hash = their_info[self.tree.PARAM_CHECKSUM]
their_hash = their_info.value
their = self.get_dir_cache(their_hash)
merged = self._merge_dirs(ancestor, our, their)
hash_info = self.tree.save_dir_info(merged)
return {hash_info.name: hash_info.value}
return self.tree.save_dir_info(merged)
+24 -21
View File
@@ -6,6 +6,7 @@ from voluptuous import Any
from dvc.dependency.local import LocalDependency
from dvc.exceptions import DvcException
from dvc.hash_info import HashInfo
from dvc.utils.serialize import LOADERS, ParseError
@@ -31,7 +32,7 @@ class ParamsDependency(LocalDependency):
else:
assert isinstance(params, dict)
self.params = list(params.keys())
info = params
info = {self.PARAM_PARAMS: params}
super().__init__(
stage,
@@ -40,46 +41,48 @@ class ParamsDependency(LocalDependency):
info=info,
)
def dumpd(self):
ret = super().dumpd()
if not self.hash_info:
ret[self.PARAM_PARAMS] = self.params
return ret
def fill_values(self, values=None):
"""Load params values dynamically."""
if not values:
return
info = {}
for param in self.params:
if param in values:
self.info[param] = values[param]
info[param] = values[param]
self.hash_info = HashInfo(self.PARAM_PARAMS, info)
def save(self):
super().save()
self.info = self.save_info()
def workspace_status(self):
status = super().workspace_status()
def status(self):
status = super().status()
if status[str(self)] == "deleted":
if status.get(str(self)) == "deleted":
return status
status = defaultdict(dict)
info = self.read_params()
info = self.hash_info.value if self.hash_info else {}
actual = self.read_params()
for param in self.params:
if param not in info.keys():
if param not in actual.keys():
st = "deleted"
elif param not in self.info:
elif param not in info:
st = "new"
elif info[param] != self.info[param]:
elif actual[param] != info[param]:
st = "modified"
else:
assert info[param] == self.info[param]
assert actual[param] == info[param]
continue
status[str(self)][param] = st
return status
def dumpd(self):
return {
self.PARAM_PATH: self.def_path,
self.PARAM_PARAMS: self.info or self.params,
}
def status(self):
return self.workspace_status()
def read_params(self):
if not self.exists:
@@ -102,7 +105,7 @@ class ParamsDependency(LocalDependency):
pass
return ret
def save_info(self):
def get_hash(self):
info = self.read_params()
missing_params = set(self.params) - set(info.keys())
@@ -113,4 +116,4 @@ class ParamsDependency(LocalDependency):
)
)
return info
return HashInfo(self.PARAM_PARAMS, info)
+6 -8
View File
@@ -49,12 +49,12 @@ class RepoDependency(LocalDependency):
rev = (d.get("rev_lock") if locked else None) or d.get("rev")
return external_repo(d["url"], rev=rev)
def _get_checksum(self, locked=True):
def _get_hash(self, locked=True):
from dvc.tree.repo import RepoTree
with self._make_repo(locked=locked) as repo:
try:
return repo.find_out_by_relpath(self.def_path).info["md5"]
return repo.find_out_by_relpath(self.def_path).hash_info
except OutputNotFoundError:
path = PathInfo(os.path.join(repo.root_dir, self.def_path))
@@ -64,16 +64,14 @@ class RepoDependency(LocalDependency):
# We are polluting our repo cache with some dir listing here
if tree.isdir(path):
return self.repo.cache.local.tree.get_hash(
path, tree=tree
).value
return self.repo.cache.local.tree.get_hash(path, tree=tree)
return tree.get_file_hash(path)
def workspace_status(self):
current_checksum = self._get_checksum(locked=True)
updated_checksum = self._get_checksum(locked=False)
current = self._get_hash(locked=True)
updated = self._get_hash(locked=False)
if current_checksum != updated_checksum:
if current != updated:
return {str(self): "update available"}
return {}
+5 -5
View File
@@ -138,13 +138,13 @@ class BaseExternalRepo:
def download_update(result):
download_results.append(result)
save_infos = []
hash_infos = []
for path in paths:
if not self.repo_tree.exists(path):
raise PathMissingError(path, self.url)
hash_info = self.repo_tree.save_info(
hash_info = self.repo_tree.get_hash(
path, download_callback=download_update
)
).to_dict()
self.local_cache.save(
path,
self.repo_tree,
@@ -152,9 +152,9 @@ class BaseExternalRepo:
save_link=False,
download_callback=download_update,
)
save_infos.append(hash_info)
hash_infos.append(hash_info)
return sum(download_results), failed, save_infos
return sum(download_results), failed, hash_infos
def get_external(self, path, dest):
"""Convenience wrapper for fetch_external and checkout."""
+10
View File
@@ -8,3 +8,13 @@ class HashInfo:
def __bool__(self):
return bool(self.value)
@classmethod
def from_dict(cls, d):
if not d:
return cls(None, None)
((name, value),) = d.items()
return cls(name, value)
def to_dict(self):
return {self.name: self.value} if self else {}
+19 -14
View File
@@ -13,6 +13,7 @@ from dvc.exceptions import (
MergeError,
RemoteCacheRequiredError,
)
from dvc.hash_info import HashInfo
from ..tree.base import BaseTree
@@ -113,7 +114,7 @@ class BaseOutput:
self.stage = stage
self.repo = stage.repo if stage else None
self.def_path = path
self.info = info
self.hash_info = HashInfo.from_dict(info)
if tree:
self.tree = tree
else:
@@ -174,10 +175,13 @@ class BaseOutput:
@property
def checksum(self):
return self.info.get(self.tree.PARAM_CHECKSUM)
return self.hash_info.value
def get_checksum(self):
return self.tree.get_hash(self.path_info).value
return self.get_hash().value
def get_hash(self):
return self.tree.get_hash(self.path_info)
@property
def is_dir_checksum(self):
@@ -187,9 +191,6 @@ class BaseOutput:
def exists(self):
return self.tree.exists(self.path_info)
def save_info(self):
return self.tree.save_info(self.path_info)
def changed_checksum(self):
return self.checksum != self.get_checksum()
@@ -267,7 +268,7 @@ class BaseOutput:
self.verify_metric()
if not self.use_cache:
self.info = self.save_info()
self.hash_info = self.get_hash()
if not self.IS_DEPENDENCY:
logger.debug(
"Output '%s' doesn't use cache. Skipping saving.", self
@@ -280,15 +281,17 @@ class BaseOutput:
logger.debug("Output '%s' didn't change. Skipping saving.", self)
return
self.info = self.save_info()
self.hash_info = self.get_hash()
def commit(self):
assert self.info
assert self.hash_info
if self.use_cache:
self.cache.save(self.path_info, self.cache.tree, self.info)
self.cache.save(
self.path_info, self.cache.tree, self.hash_info.to_dict()
)
def dumpd(self):
ret = copy(self.info)
ret = copy(self.hash_info.to_dict())
ret[self.PARAM_PATH] = self.def_path
if self.IS_DEPENDENCY:
@@ -337,7 +340,7 @@ class BaseOutput:
return self.cache.checkout(
self.path_info,
self.info,
self.hash_info.to_dict(),
force=force,
progress_callback=progress_callback,
relink=relink,
@@ -535,11 +538,13 @@ class BaseOutput:
if ancestor:
self._check_can_merge(ancestor)
ancestor_info = ancestor.info
ancestor_info = ancestor.hash_info
else:
ancestor_info = None
self._check_can_merge(self)
self._check_can_merge(other)
self.info = self.cache.merge(ancestor_info, self.info, other.info)
self.hash_info = self.cache.merge(
ancestor_info, self.hash_info, other.hash_info
)
+5 -3
View File
@@ -6,6 +6,7 @@ from itertools import chain
from funcy import get_in, lcat, project
from dvc import dependency, output
from dvc.hash_info import HashInfo
from . import PipelineStage, Stage, loads_from
from .exceptions import StageNameUnspecified, StageNotFound
@@ -42,9 +43,10 @@ class StageLoader(Mapping):
if isinstance(item, dependency.ParamsDependency):
item.fill_values(get_in(lock_data, [stage.PARAM_PARAMS, path]))
continue
item.info = get_in(checksums, [key, path], {})
item.info = item.info.copy()
item.info.pop("path", None)
info = get_in(checksums, [key, path], {})
info = info.copy()
info.pop("path", None)
item.hash_info = HashInfo.from_dict(info)
@classmethod
def load_stage(cls, dvcfile, name, stage_data, lock_data=None):
+6 -1
View File
@@ -138,7 +138,12 @@ def to_single_stage_lockfile(stage: "Stage") -> dict:
params, deps = split_params_deps(stage)
deps, outs = [
[
OrderedDict([(PARAM_PATH, item.def_path), *item.info.items()])
OrderedDict(
[
(PARAM_PATH, item.def_path),
*item.hash_info.to_dict().items(),
]
)
for item in sort_by_path(items)
]
for items in [deps, stage.outs]
-4
View File
@@ -294,10 +294,6 @@ class BaseTree:
return "".join(parts)
def save_info(self, path_info, **kwargs):
hash_info = self.get_hash(path_info, **kwargs)
return {hash_info.name: hash_info.value}
def _calculate_hashes(self, file_infos):
file_infos = list(file_infos)
with Tqdm(
+3 -2
View File
@@ -18,6 +18,7 @@ from dvc.exceptions import (
OverlappingOutputPathsError,
RecursiveAddingWhileUsingFilename,
)
from dvc.hash_info import HashInfo
from dvc.main import main
from dvc.output.base import OutputAlreadyTrackedError, OutputIsStageFileError
from dvc.repo import Repo as DvcRepo
@@ -42,7 +43,7 @@ def test_add(tmp_dir, dvc):
assert len(stage.outs) == 1
assert len(stage.deps) == 0
assert stage.cmd is None
assert stage.outs[0].info["md5"] == md5
assert stage.outs[0].hash_info == HashInfo("md5", md5)
assert stage.md5 is None
assert load_yaml("foo.dvc") == {
@@ -71,7 +72,7 @@ def test_add_directory(tmp_dir, dvc):
assert len(stage.deps) == 0
assert len(stage.outs) == 1
md5 = stage.outs[0].info["md5"]
md5 = stage.outs[0].hash_info.value
dir_info = dvc.cache.local.load_dir_cache(md5)
for info in dir_info:
+1 -1
View File
@@ -76,7 +76,7 @@ def test_uncached_outs_are_cached(tmp_dir, dvc, run_copy):
name="copy-foo-bar",
)
with dvc.state:
stage.outs[0].info = stage.outs[0].save_info()
stage.outs[0].hash_info = stage.outs[0].get_hash()
assert os.path.exists(relpath(stage.outs[0].cache_path))
+1 -1
View File
@@ -226,7 +226,7 @@ def test_repotree_cache_save(tmp_dir, dvc, scm, erepo_dir, local_cloud):
cache = dvc.cache.local
with cache.tree.state:
path_info = PathInfo(erepo_dir / "dir")
hash_info = cache.tree.save_info(path_info)
hash_info = cache.tree.get_hash(path_info).to_dict()
cache.save(path_info, tree, hash_info)
for hash_ in expected:
+8 -8
View File
@@ -32,17 +32,17 @@ def test_loads_params(dvc):
assert isinstance(deps[0], ParamsDependency)
assert deps[0].def_path == ParamsDependency.DEFAULT_PARAMS_FILE
assert deps[0].params == ["foo", "bar"]
assert deps[0].info == {}
assert not deps[0].hash_info
assert isinstance(deps[1], ParamsDependency)
assert deps[1].def_path == "a_file"
assert deps[1].params == ["baz", "bat", "foobar"]
assert deps[1].info == {}
assert not deps[1].hash_info
assert isinstance(deps[2], ParamsDependency)
assert deps[2].def_path == "b_file"
assert deps[2].params == ["cat"]
assert deps[2].info == {}
assert not deps[2].hash_info
@pytest.mark.parametrize("params", [[3], [{"b_file": "cat"}]])
@@ -58,7 +58,7 @@ def test_loadd_from(dvc):
assert isinstance(deps[0], ParamsDependency)
assert deps[0].def_path == ParamsDependency.DEFAULT_PARAMS_FILE
assert deps[0].params == list(PARAMS.keys())
assert deps[0].info == PARAMS
assert deps[0].hash_info.value == PARAMS
def test_dumpd_with_info(dvc):
@@ -119,17 +119,17 @@ def test_read_params_toml(tmp_dir, dvc):
assert dep.read_params() == {"some.path.foo": ["val1", "val2"]}
def test_save_info_missing_config(dvc):
def test_get_hash_missing_config(dvc):
dep = ParamsDependency(Stage(dvc), None, ["foo"])
with pytest.raises(MissingParamsError):
dep.save_info()
dep.get_hash()
def test_save_info_missing_param(tmp_dir, dvc):
def test_get_hash_missing_param(tmp_dir, dvc):
tmp_dir.gen(DEFAULT_PARAMS_FILE, "bar: baz")
dep = ParamsDependency(Stage(dvc), None, ["foo"])
with pytest.raises(MissingParamsError):
dep.save_info()
dep.get_hash()
@pytest.mark.regression_4184
+3 -3
View File
@@ -39,7 +39,7 @@ def test_load_from_pipeline(dvc, out_type, type_test_func):
assert out.def_path == f"file{i}"
assert out.use_cache == (out.def_path in cached_outs)
assert out.persist == (out.def_path in persisted_outs)
assert out.info == {}
assert not out.hash_info
assert type_test_func(out)
@@ -57,7 +57,7 @@ def test_load_from_pipeline_accumulates_flag(dvc):
for out in outs:
assert isinstance(out, LocalOutput)
assert not out.plot and not out.metric
assert out.info == {}
assert not out.hash_info
assert outs[0].use_cache and not outs[0].persist
assert not outs[1].use_cache and outs[1].persist
@@ -71,7 +71,7 @@ def test_load_remote_files_from_pipeline(dvc):
assert isinstance(out, S3Output)
assert not out.plot and out.metric
assert not out.persist
assert out.info == {}
assert not out.hash_info
@pytest.mark.parametrize("typ", [None, "", "illegal"])
+15 -14
View File
@@ -5,6 +5,7 @@ from itertools import chain
import pytest
from dvc.dvcfile import PIPELINE_FILE, Dvcfile
from dvc.hash_info import HashInfo
from dvc.stage import PipelineStage, create_stage
from dvc.stage.loader import StageLoader
from dvc.stage.serialize import split_params_deps
@@ -30,12 +31,12 @@ def test_fill_from_lock_deps_outs(dvc, lock_data):
)
for item in chain(stage.deps, stage.outs):
assert not item.checksum and not item.info
assert not item.checksum and not item.hash_info
StageLoader.fill_from_lock(stage, lock_data)
assert stage.deps[0].info == {"md5": "foo_checksum"}
assert stage.outs[0].info == {"md5": "bar_checksum"}
assert stage.deps[0].hash_info == HashInfo("md5", "foo_checksum")
assert stage.outs[0].hash_info == HashInfo("md5", "bar_checksum")
def test_fill_from_lock_params(dvc, lock_data):
@@ -64,12 +65,14 @@ def test_fill_from_lock_params(dvc, lock_data):
params_deps = split_params_deps(stage)[0]
assert set(params_deps[0].params) == {"lorem", "lorem.ipsum"}
assert set(params_deps[1].params) == {"ipsum", "foobar"}
assert not params_deps[0].info
assert not params_deps[1].info
assert not params_deps[0].hash_info
assert not params_deps[1].hash_info
StageLoader.fill_from_lock(stage, lock_data)
assert params_deps[0].info == lock_data["params"]["params.yaml"]
assert params_deps[1].info == lock_data["params"]["myparams.yaml"]
assert params_deps[0].hash_info.value == lock_data["params"]["params.yaml"]
assert (
params_deps[1].hash_info.value == lock_data["params"]["myparams.yaml"]
)
def test_fill_from_lock_missing_params_section(dvc, lock_data):
@@ -83,7 +86,7 @@ def test_fill_from_lock_missing_params_section(dvc, lock_data):
)
params_deps = split_params_deps(stage)[0]
StageLoader.fill_from_lock(stage, lock_data)
assert not params_deps[0].info and not params_deps[1].info
assert not params_deps[0].hash_info and not params_deps[1].hash_info
def test_fill_from_lock_missing_checksums(dvc, lock_data):
@@ -97,8 +100,8 @@ def test_fill_from_lock_missing_checksums(dvc, lock_data):
StageLoader.fill_from_lock(stage, lock_data)
assert stage.deps[0].info == {"md5": "foo_checksum"}
assert stage.outs[0].info == {"md5": "bar_checksum"}
assert stage.deps[0].hash_info == HashInfo("md5", "foo_checksum")
assert stage.outs[0].hash_info == HashInfo("md5", "bar_checksum")
assert not stage.deps[1].checksum and not stage.outs[1].checksum
@@ -110,9 +113,7 @@ def test_fill_from_lock_use_appropriate_checksum(dvc, lock_data):
deps=["s3://dvc-temp/foo"],
outs=["bar"],
)
lock_data["deps"] = [
{"path": "s3://dvc-temp/foo", "md5": "high five", "etag": "e-tag"}
]
lock_data["deps"] = [{"path": "s3://dvc-temp/foo", "etag": "e-tag"}]
StageLoader.fill_from_lock(stage, lock_data)
assert stage.deps[0].checksum == "e-tag"
assert stage.outs[0].checksum == "bar_checksum"
@@ -183,7 +184,7 @@ def test_load_stage_with_params(dvc, stage_data, lock_data):
params, deps = split_params_deps(stage)
assert deps[0].def_path == "foo" and stage.outs[0].def_path == "bar"
assert params[0].def_path == "params.yaml"
assert params[0].info == {"lorem": "ipsum"}
assert params[0].hash_info == HashInfo("params", {"lorem": "ipsum"})
assert deps[0].checksum == "foo_checksum"
assert stage.outs[0].checksum == "bar_checksum"
@@ -4,6 +4,7 @@ import pytest
from voluptuous import Schema as _Schema
from dvc.dvcfile import PIPELINE_FILE
from dvc.hash_info import HashInfo
from dvc.schema import LOCK_FILE_STAGE_SCHEMA, LOCKFILE_SCHEMA
from dvc.stage import PipelineStage, create_stage
from dvc.stage.serialize import DEFAULT_PARAMS_FILE, to_lockfile
@@ -30,7 +31,7 @@ def test_lock(dvc):
def test_lock_deps(dvc):
stage = create_stage(PipelineStage, dvc, deps=["input"], **kwargs)
stage.deps[0].info = {"md5": "md-five"}
stage.deps[0].hash_info = HashInfo("md5", "md-five")
assert to_single_stage_lockfile(stage) == OrderedDict(
[
("cmd", "command"),
@@ -43,8 +44,8 @@ def test_lock_deps_order(dvc):
stage = create_stage(
PipelineStage, dvc, deps=["input1", "input0"], **kwargs
)
stage.deps[0].info = {"md5": "md-one1"}
stage.deps[1].info = {"md5": "md-zer0"}
stage.deps[0].hash_info = HashInfo("md5", "md-one1")
stage.deps[1].hash_info = HashInfo("md5", "md-zer0")
assert to_single_stage_lockfile(stage) == OrderedDict(
[
("cmd", "command"),
@@ -63,7 +64,9 @@ def test_lock_params(dvc):
stage = create_stage(
PipelineStage, dvc, params=["lorem.ipsum", "abc"], **kwargs
)
stage.deps[0].info = {"lorem.ipsum": {"lorem1": 1, "lorem2": 2}, "abc": 3}
stage.deps[0].hash_info = HashInfo(
"params", {"lorem.ipsum": {"lorem1": 1, "lorem2": 2}, "abc": 3}
)
assert to_single_stage_lockfile(stage)["params"][
DEFAULT_PARAMS_FILE
] == OrderedDict([("abc", 3), ("lorem.ipsum", {"lorem1": 1, "lorem2": 2})])
@@ -81,9 +84,15 @@ def test_lock_params_file_sorted(dvc):
],
**kwargs
)
stage.deps[0].info = {"lorem.ipsum": {"lorem1": 1, "lorem2": 2}, "abc": 3}
stage.deps[1].info = {"foo": ["f", "o", "o"], "foobar": "foobar"}
stage.deps[2].info = {"bar": ["b", "a", "r"], "barr": "barr"}
stage.deps[0].hash_info = HashInfo(
"params", {"lorem.ipsum": {"lorem1": 1, "lorem2": 2}, "abc": 3}
)
stage.deps[1].hash_info = HashInfo(
"params", {"foo": ["f", "o", "o"], "foobar": "foobar"}
)
stage.deps[2].hash_info = HashInfo(
"params", {"bar": ["b", "a", "r"], "barr": "barr"}
)
assert to_single_stage_lockfile(stage)["params"] == OrderedDict(
[
(
@@ -114,7 +123,7 @@ def test_lock_params_no_values_filled(dvc):
@pytest.mark.parametrize("typ", ["plots", "metrics", "outs"])
def test_lock_outs(dvc, typ):
stage = create_stage(PipelineStage, dvc, **{typ: ["input"]}, **kwargs)
stage.outs[0].info = {"md5": "md-five"}
stage.outs[0].hash_info = HashInfo("md5", "md-five")
assert to_single_stage_lockfile(stage) == OrderedDict(
[
("cmd", "command"),
@@ -128,8 +137,8 @@ def test_lock_outs_order(dvc, typ):
stage = create_stage(
PipelineStage, dvc, **{typ: ["input1", "input0"]}, **kwargs
)
stage.outs[0].info = {"md5": "md-one1"}
stage.outs[1].info = {"md5": "md-zer0"}
stage.outs[0].hash_info = HashInfo("md5", "md-one1")
stage.outs[1].hash_info = HashInfo("md5", "md-zer0")
assert to_single_stage_lockfile(stage) == OrderedDict(
[
("cmd", "command"),
@@ -148,7 +157,7 @@ def test_dump_nondefault_hash(dvc):
stage = create_stage(
PipelineStage, dvc, deps=["s3://dvc-temp/file"], **kwargs
)
stage.deps[0].info = {"md5": "value"}
stage.deps[0].hash_info = HashInfo("md5", "value")
assert to_single_stage_lockfile(stage) == OrderedDict(
[
("cmd", "command"),
@@ -164,30 +173,6 @@ def test_dump_nondefault_hash(dvc):
)
def test_dump_multiple_hashes(dvc):
stage = create_stage(
PipelineStage, dvc, deps=["s3://dvc-temp/file"], **kwargs
)
stage.deps[0].info = {"etag": "etag-value", "md5": "md5-value"}
assert to_single_stage_lockfile(stage) == OrderedDict(
[
("cmd", "command"),
(
"deps",
[
OrderedDict(
[
("path", "s3://dvc-temp/file"),
("etag", "etag-value"),
("md5", "md5-value"),
]
)
],
),
]
)
def test_order(dvc):
stage = create_stage(
PipelineStage,
@@ -199,9 +184,9 @@ def test_order(dvc):
)
params, deps = split_params_deps(stage)
deps[0].info = {"md5": "md-five"}
params[0].info = {"foo-param": "value"}
stage.outs[0].info = {"md5": "md5-output"}
deps[0].hash_info = HashInfo("md5", "md-five")
params[0].hash_info = HashInfo("params", {"foo-param": "value"})
stage.outs[0].hash_info = HashInfo("md5", "md5-output")
assert to_single_stage_lockfile(stage) == OrderedDict(
[
@@ -215,7 +200,7 @@ def test_order(dvc):
def test_to_lockfile(dvc):
stage = create_stage(PipelineStage, dvc, deps=["input"], **kwargs)
stage.deps[0].info = {"md5": "md-five"}
stage.deps[0].hash_info = HashInfo("md5", "md-five")
entry = to_lockfile(stage)
assert len(entry) == 1
_Schema(LOCKFILE_SCHEMA)(entry)