Repository: con/tinuous (language: python)

Issue:
fetching releases failed
With the following config
```
(tinuous-dev) datalad@smaug:/mnt/datasets/datalad/ci/git-annex$ cat config.yml 
repo: datalad/git-annex
vars:
  common_path: 'builds/{year}//{month}//{day}/{type}-{type_id}/{build_commit[:8]}/{wf_name}-{number}-{common_status}/'
ci:
  github:
    path: '{common_path}'
    # place artifacts along with the logs
    artifacts_path: '{common_path}'
    # but releases separately and without subdataset
    releases_path: 'releases/'
    workflows:
    - build-macos.yaml
    - build-ubuntu.yaml
    - build-windows.yaml
since: 2021-04-27T00:00:00Z
types: [cron, pr, push]
datalad:
  enabled: true
  cfg_proc: text2git
```

eventually `tinuous fetch` (as of 0.2.0-30-g0215fbb) crashed with

```
2021-06-08T12:03:28-0400 [INFO    ] tinuous Fetching releases newer than 2021-04-27 00:00:00+00:00
2021-06-08T12:03:28-0400 [INFO    ] tinuous Found release 8.20210310
2021-06-08T12:03:28-0400 [INFO    ] tinuous Found release 8.20210428
Traceback (most recent call last):
  File "/home/datalad/miniconda3/envs/tinuous-dev/bin/tinuous", line 33, in <module>
    sys.exit(load_entry_point('tinuous', 'console_scripts', 'tinuous')())
  File "/home/datalad/miniconda3/envs/tinuous-dev/lib/python3.9/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/home/datalad/miniconda3/envs/tinuous-dev/lib/python3.9/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/home/datalad/miniconda3/envs/tinuous-dev/lib/python3.9/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/datalad/miniconda3/envs/tinuous-dev/lib/python3.9/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/datalad/miniconda3/envs/tinuous-dev/lib/python3.9/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/home/datalad/miniconda3/envs/tinuous-dev/lib/python3.9/site-packages/click/decorators.py", line 33, in new_func
    return f(get_current_context().obj, *args, **kwargs)
  File "/mnt/datasets/datalad/ci/tinuous/src/tinuous/__main__.py", line 128, in fetch
    path = asset.expand_path(cicfg.releases_path, cfg.vars)
  File "/mnt/datasets/datalad/ci/tinuous/src/tinuous/github.py", line 365, in expand_path
    return expand_template(path_template, self.path_fields(), vars)
  File "/mnt/datasets/datalad/ci/tinuous/src/tinuous/util.py", line 53, in expand_template
    expanded_vars[name] = fstring(tmplt, **fields, **expanded_vars)
  File "/mnt/datasets/datalad/ci/tinuous/src/tinuous/util.py", line 58, in fstring
    return cast(str, eval(f"f{s!r}", {}, kwargs))
  File "<string>", line 1, in <module>
NameError: name 'wf_name' is not defined
```

seems because we expand all variables unconditionally of either they are used or not in the given path.  I guess we could just skip the variables which fail to expand ? (I guess would need at least some .debug level log message to be able to undig that later)  or do you see a better way to resolve it @jwodder ?


Discussion / hints:

Fix patch:
diff --git a/src/tinuous/util.py b/src/tinuous/util.py
index 0ded703..fd62238 100644
--- a/src/tinuous/util.py
+++ b/src/tinuous/util.py
@@ -4,8 +4,9 @@
 import os
 from pathlib import Path
 import re
+from string import Formatter
 import subprocess
-from typing import Dict, Iterator, cast
+from typing import Any, Dict, Iterator, Mapping, Optional, Sequence, Union
 
 import requests
 
@@ -46,17 +47,88 @@ def iterfiles(dirpath: Path) -> Iterator[Path]:
                 yield p
 
 
+class LazySlicingFormatter(Formatter):
+    """
+    A `string.Formatter` subclass that:
+
+    - accepts a second set of format kwargs that can refer to the main kwargs
+      or each other and are only templated as needed
+    - supports indexing strings & other sequences with slices
+    """
+
+    def __init__(self, var_defs: Dict[str, str]):
+        self.var_defs: Dict[str, str] = var_defs
+        self.expanded_vars: Dict[str, str] = {}
+        super().__init__()
+
+    def get_value(
+        self, key: Union[int, str], args: Sequence[Any], kwargs: Mapping[str, Any]
+    ) -> Any:
+        if isinstance(key, int):
+            return args[key]
+        elif key in kwargs:
+            return kwargs[key]
+        elif key in self.expanded_vars:
+            return self.expanded_vars[key]
+        elif key in self.var_defs:
+            self.expanded_vars[key] = self.format(self.var_defs[key], **kwargs)
+            return self.expanded_vars[key]
+        else:
+            raise KeyError(key)
+
+    def get_field(
+        self, field_name: str, args: Sequence[Any], kwargs: Mapping[str, Any]
+    ) -> Any:
+        m = re.match(r"\w+", field_name)
+        assert m, f"format field name {field_name!r} does not start with arg_name"
+        key: Union[int, str] = m.group()
+        if key.isdigit():
+            key = int(key)
+        obj = self.get_value(key, args, kwargs)
+        s = field_name[m.end() :]
+        while s:
+            m = re.match(r"\.(?P<attr>\w+)|\[(?P<index>[^]]+)\]", s)
+            assert m, f"format field name {field_name!r} has invalid attr/index"
+            s = s[m.end() :]
+            attr, index = m.group("attr", "index")
+            if attr is not None:
+                obj = getattr(obj, attr)
+            else:
+                assert index is not None  # type: ignore[unreachable]
+                try:
+                    sl = parse_slice(index)
+                except ValueError:
+                    if index.isdigit():
+                        obj = obj[int(index)]
+                    else:
+                        obj = obj[index]
+                else:
+                    obj = obj[sl]
+        return obj, key
+
+
 def expand_template(
     template_str: str, fields: Dict[str, str], vars: Dict[str, str]
 ) -> str:
-    expanded_vars: Dict[str, str] = {}
-    for name, tmplt in vars.items():
-        expanded_vars[name] = fstring(tmplt, **fields, **expanded_vars)
-    return fstring(template_str, **fields, **expanded_vars)
+    return LazySlicingFormatter(vars).format(template_str, **fields)
+
+
+SLICE_RGX = re.compile(r"(?P<start>-?\d+)?:(?P<stop>-?\d+)?(?::(?P<step>-?\d+)?)?")
 
 
-def fstring(s: str, **kwargs: str) -> str:
-    return cast(str, eval(f"f{s!r}", {}, kwargs))
+def parse_slice(s: str) -> slice:
+    if m := SLICE_RGX.fullmatch(s):
+        s_start, s_stop, s_step = m.group("start", "stop", "step")
+        start: Optional[int] = None if s_start is None else int(s_start)
+        stop: Optional[int] = None if s_stop is None else int(s_stop)
+        step: Optional[int]
+        if s_step is None or s_step == "":
+            step = None
+        else:
+            step = int(s_step)
+        return slice(start, stop, step)
+    else:
+        raise ValueError(s)
 
 
 def get_github_token() -> str:
diff --git a/tox.ini b/tox.ini
index 03fdb72..f6a9bad 100644
--- a/tox.ini
+++ b/tox.ini
@@ -8,6 +8,7 @@ minversion = 3.3.0
 deps =
     pytest~=6.0
     pytest-cov~=2.0
+    pytest-mock~=3.0
 commands =
     # Basic smoketest:
     tinuous --help


Test patch:
diff --git a/test/test_util.py b/test/test_util.py
index fe2e273..4ccc510 100644
--- a/test/test_util.py
+++ b/test/test_util.py
@@ -1,4 +1,8 @@
-from tinuous.util import expand_template
+from types import SimpleNamespace
+from typing import Any, Dict
+import pytest
+from pytest_mock import MockerFixture
+from tinuous.util import LazySlicingFormatter, expand_template, parse_slice
 
 
 def test_expand_template() -> None:
@@ -24,3 +28,68 @@ def test_expand_template_sliced() -> None:
         )
         == "1234567/A test"
     )
+
+
+def test_expand_template_unused_bad() -> None:
+    assert (
+        expand_template(
+            "{cleesh}",
+            {"description": "A test commit"},
+            {"bad": "{undefined}", "cleesh": "{description[:6]}"},
+        )
+        == "A test"
+    )
+
+
+@pytest.mark.parametrize(
+    "s,sl",
+    [
+        (":", slice(None)),
+        ("::", slice(None)),
+        ("23:", slice(23, None)),
+        ("23::", slice(23, None)),
+        (":42", slice(42)),
+        (":42:", slice(42)),
+        ("23:42", slice(23, 42)),
+        ("23:42:", slice(23, 42)),
+        ("::5", slice(None, None, 5)),
+        ("23::5", slice(23, None, 5)),
+        (":42:5", slice(None, 42, 5)),
+        ("23:42:5", slice(23, 42, 5)),
+        ("-23:-42:-5", slice(-23, -42, -5)),
+    ],
+)
+def test_parse_slice(s: str, sl: slice) -> None:
+    assert parse_slice(s) == sl
+
+
+@pytest.mark.parametrize(
+    "fmt,args,kwargs,result",
+    [
+        ("{0}", ["foo"], {}, "foo"),
+        (
+            "{foo.bar.baz}",
+            [],
+            {"foo": SimpleNamespace(bar=SimpleNamespace(baz="quux"))},
+            "quux",
+        ),
+        ("{foo[1][2]}", [], {"foo": ["abc", "def", "ghi"]}, "f"),
+        ("{foo[bar][baz]}", [], {"foo": {"bar": {"baz": "quux"}}}, "quux"),
+    ],
+)
+def test_lazy_slicing_formatter_basics(
+    fmt: str, args: list, kwargs: Dict[str, Any], result: str
+) -> None:
+    assert LazySlicingFormatter({}).format(fmt, *args, **kwargs) == result
+
+
+def test_lazy_slicing_formatter_undef_key() -> None:
+    with pytest.raises(KeyError):
+        LazySlicingFormatter({}).format("{foo}", bar=42)
+
+
+def test_lazy_slicing_formatter_var_reuse(mocker: MockerFixture) -> None:
+    fmter = LazySlicingFormatter({"foo": "bar"})
+    spy = mocker.spy(fmter, "format")
+    assert fmter.format("-{foo}-{foo}-") == "-bar-bar-"
+    assert spy.call_args_list == [mocker.call("-{foo}-{foo}-"), mocker.call("bar")]