parametrization: improve set error message (#4923)

dvc/parsing/__init__.py

@@ -9,9 +9,10 @@ from typing import TYPE_CHECKING, List, Set
 from funcy import join
 
 from dvc.dependency.param import ParamsDependency
+from dvc.exceptions import DvcException
 from dvc.path_info import PathInfo
 
-from .context import Context
+from .context import Context, SetError
 
 if TYPE_CHECKING:
     from dvc.repo import Repo
@@ -30,6 +31,17 @@ SET_KWD = "set"
 DEFAULT_SENTINEL = object()
 
 
+class ResolveError(DvcException):
+    pass
+
+
+def _reraise_err(exc_cls, *args, from_exc=None):
+    err = exc_cls(*args)
+    if from_exc and logger.isEnabledFor(logging.DEBUG):
+        raise err from from_exc
+    raise err
+
+
 class DataResolver:
     def __init__(self, repo: "Repo", wdir: PathInfo, d: dict):
 
@@ -76,7 +88,9 @@ class DataResolver:
     def _resolve_entry(self, name: str, definition):
         context = Context.clone(self.global_ctx)
         if FOREACH_KWD in definition:
-            self.set_context_from(context, definition.get(SET_KWD, {}))
+            self.set_context_from(
+                context, definition.get(SET_KWD, {}), source=[name, "set"]
+            )
             assert IN_KWD in definition
             return self._foreach(
                 context, name, definition[FOREACH_KWD], definition[IN_KWD]
@@ -91,7 +105,9 @@ class DataResolver:
 
     def _resolve_stage(self, context: Context, name: str, definition) -> dict:
         definition = deepcopy(definition)
-        self.set_context_from(context, definition.pop(SET_KWD, {}))
+        self.set_context_from(
+            context, definition.pop(SET_KWD, {}), source=[name, "set"]
+        )
         wdir = self._resolve_wdir(context, definition.get(WDIR_KWD))
         if self.wdir != wdir:
             logger.debug(
@@ -153,6 +169,10 @@ class DataResolver:
         return join(gen)
 
     @classmethod
-    def set_context_from(cls, context: Context, to_set):
-        for key, value in to_set.items():
-            context.set(key, value)
+    def set_context_from(cls, context: Context, to_set, source=None):
+        try:
+            for key, value in to_set.items():
+                src_set = [*(source or []), key]
+                context.set(key, value, source=".".join(src_set))
+        except SetError as exc:
+            _reraise_err(ResolveError, str(exc), from_exc=exc)

dvc/parsing/context.py

@@ -11,6 +11,7 @@ from funcy import identity, rpartial
 
 from dvc.exceptions import DvcException
 from dvc.parsing.interpolate import (
+    ParseError,
     get_expression,
     get_matches,
     is_exact_string,
@@ -26,6 +27,10 @@ class ContextError(DvcException):
     pass
 
 
+class SetError(ContextError):
+    pass
+
+
 class MergeError(ContextError):
     def __init__(self, key, new, into):
         self.key = key
@@ -66,6 +71,7 @@ def _merge(into, update, overwrite):
 class Meta:
     source: Optional[str] = None
     dpaths: List[str] = field(default_factory=list)
+    local: bool = True
 
     @staticmethod
     def update_path(meta: "Meta", path: Union[str, int]):
@@ -129,11 +135,17 @@ class Container(Node, ABC):
 
     def _convert(self, key, value):
         meta = Meta.update_path(self.meta, key)
+        return self._convert_with_meta(value, meta)
+
+    @staticmethod
+    def _convert_with_meta(value, meta: Meta = None):
         if value is None or isinstance(value, PRIMITIVES):
+            assert meta
             return Value(value, meta=meta)
         elif isinstance(value, Node):
             return value
         elif isinstance(value, (list, dict)):
+            assert meta
             container = CtxDict if isinstance(value, dict) else CtxList
             return container(value, meta=meta)
         else:
@@ -255,6 +267,9 @@ class Context(CtxDict):
         if not self._track:
             return
 
+        if node.meta and node.meta.local:
+            return
+
         for source, keys in node.get_sources().items():
             if not source:
                 continue
@@ -294,7 +309,7 @@ class Context(CtxDict):
         _, ext = os.path.splitext(file)
         loader = LOADERS[ext]
 
-        meta = Meta(source=file)
+        meta = Meta(source=file, local=False)
         return cls(loader(file, tree=tree), meta=meta)
 
     def merge_from(self, tree, path, overwrite=False):
@@ -310,7 +325,7 @@ class Context(CtxDict):
         """Clones given context."""
         return cls(deepcopy(ctx.data))
 
-    def set(self, key, value):
+    def set(self, key, value, source=None):
         """
         Sets a value, either non-interpolated values to a key,
         or an interpolated string after resolving it.
@@ -320,6 +335,13 @@ class Context(CtxDict):
         >>> c
         {'foo': 'foo', 'bar': [1, 2], 'lorem': {'a': 'z'}, 'foobar': [1, 2]}
         """
+        try:
+            self._set(key, value, source)
+        except (ParseError, ValueError, ContextError) as exc:
+            sp = "\n" if isinstance(exc, ParseError) else " "
+            raise SetError(f"Failed to set '{source}':{sp}{str(exc)}") from exc
+
+    def _set(self, key, value, source):
         if key in self:
             raise ValueError(f"Cannot set '{key}', key already exists")
         if isinstance(value, str):
@@ -328,7 +350,7 @@ class Context(CtxDict):
         elif isinstance(value, (Sequence, Mapping)):
             self._check_not_nested_collection(key, value)
             self._check_interpolation_collection(key, value)
-        self[key] = value
+        self[key] = self._convert_with_meta(value, Meta(source=source))
 
     def resolve(self, src, unwrap=True):
         """Recursively resolves interpolation and returns resolved data.
@@ -399,7 +421,7 @@ class Context(CtxDict):
         if matches and not is_exact_string(value, matches):
             raise ValueError(
                 f"Cannot set '{key}', "
-                "joining string with interpolated string"
+                "joining string with interpolated string "
                 "is not supported"
             )
 

tests/unit/test_context.py

@@ -221,20 +221,35 @@ def test_load_from(mocker):
     file = "params.yaml"
     c = Context.load_from(object(), file)
 
-    assert asdict(c["x"].meta) == {"source": file, "dpaths": ["x"]}
-    assert asdict(c["foo"].meta) == {"source": file, "dpaths": ["foo"]}
-    assert asdict(c["x"]["y"].meta) == {"source": file, "dpaths": ["x", "y"]}
+    assert asdict(c["x"].meta) == {
+        "source": file,
+        "dpaths": ["x"],
+        "local": False,
+    }
+    assert asdict(c["foo"].meta) == {
+        "source": file,
+        "local": False,
+        "dpaths": ["foo"],
+    }
+    assert asdict(c["x"]["y"].meta) == {
+        "source": file,
+        "dpaths": ["x", "y"],
+        "local": False,
+    }
     assert asdict(c["x"]["y"]["z"].meta) == {
         "source": file,
         "dpaths": ["x", "y", "z"],
+        "local": False,
     }
     assert asdict(c["x"]["lst"].meta) == {
         "source": file,
         "dpaths": ["x", "lst"],
+        "local": False,
     }
     assert asdict(c["x"]["lst"][0].meta) == {
         "source": file,
         "dpaths": ["x", "lst", "0"],
+        "local": False,
     }
 
 

tests/unit/test_stage_resolver.py

@@ -2,7 +2,7 @@ from math import pi
 
 import pytest
 
-from dvc.parsing import DataResolver
+from dvc.parsing import DataResolver, ResolveError
 from dvc.parsing.context import Context, Value
 
 TEMPLATED_DVC_YAML_DATA = {
@@ -75,17 +75,24 @@ def test_set():
 )
 def test_set_nested_coll(coll):
     context = Context(CONTEXT_DATA)
-    with pytest.raises(ValueError, match="Cannot set 'item', has nested"):
+    with pytest.raises(ResolveError) as exc_info:
         DataResolver.set_context_from(context, {"thresh": 10, "item": coll})
 
+    assert (
+        str(exc_info.value) == "Failed to set 'item': Cannot set 'item', "
+        "has nested dict/list"
+    )
+
 
 def test_set_already_exists():
     context = Context({"item": "foo"})
-    with pytest.raises(
-        ValueError, match="Cannot set 'item', key already exists"
-    ):
+    with pytest.raises(ResolveError) as exc_info:
         DataResolver.set_context_from(context, {"item": "bar"})
 
+    assert (
+        str(exc_info.value) == "Failed to set 'item': Cannot set 'item', "
+        "key already exists"
+    )
     assert context["item"] == Value("foo")
 
 
@@ -94,11 +101,15 @@ def test_set_already_exists():
 )
 def test_set_collection_interpolation(coll):
     context = Context(CONTEXT_DATA)
-    with pytest.raises(
-        ValueError, match="Cannot set 'item', having interpolation inside"
-    ):
+    with pytest.raises(ResolveError) as exc_info:
         DataResolver.set_context_from(context, {"thresh": 10, "item": coll})
 
+    assert (
+        str(exc_info.value) == "Failed to set 'item': Cannot set 'item', "
+        f"having interpolation inside '{type(coll).__name__}' "
+        "is not supported."
+    )
+
 
 def test_set_interpolated_string():
     context = Context(CONTEXT_DATA)
@@ -168,8 +179,10 @@ def test_set_ladder():
 )
 def test_set_multiple_interpolations(value):
     context = Context(CONTEXT_DATA)
-    with pytest.raises(
-        ValueError,
-        match=r"Cannot set 'item', joining string with interpolated string",
-    ):
+    with pytest.raises(ResolveError,) as exc_info:
         DataResolver.set_context_from(context, {"thresh": 10, "item": value})
+
+    assert str(exc_info.value) == (
+        "Failed to set 'item': Cannot set 'item', "
+        "joining string with interpolated string is not supported"
+    )