summon: pair-programming adjustments

This commit is contained in:
Mr. Outis
2019-12-25 00:42:55 -06:00
parent 7a58901ec6
commit 0658622ca2
2 changed files with 84 additions and 63 deletions
+60 -53
View File
@@ -10,7 +10,7 @@ except ImportError:
from contextlib import GeneratorContextManager as GCM
import ruamel.yaml
from voluptuous import Schema, Optional, Invalid, ALLOW_EXTRA
from voluptuous import Schema, Required, Invalid
from dvc.utils.compat import urlparse, builtin_str
from dvc.repo import Repo
@@ -18,22 +18,21 @@ from dvc.exceptions import SummonError, FileMissingError
from dvc.external_repo import external_repo
summon_schema = Schema(
SUMMON_SCHEMA = Schema(
{
"objects": [
Required("objects"): [
{
"name": str,
"summon": {
"type": "python",
"call": str,
Optional("args"): dict,
Optional("deps"): [str],
Required("name"): str,
"meta": dict,
Required("summon"): {
Required("type"): "python",
Required("call"): str,
"args": dict,
"deps": [str],
},
}
]
},
required=True,
extra=ALLOW_EXTRA,
)
@@ -97,26 +96,20 @@ def _make_repo(repo_url, rev=None):
yield repo
def summon(name, fname="dvcsummon.yaml", args=None, repo=None, rev=None):
# TODO: Write a meaningful docstring about `summon`
def summon(name, repo=None, rev=None, summon_file="dvcsummon.yaml", args=None):
"""Instantiate an object described in the summon file."""
with _make_repo(repo, rev=rev) as _repo:
try:
path = os.path.join(_repo.root_dir, summon_file)
obj = _get_object_from_summon_file(name, path)
info = obj["summon"]
except SummonError as exc:
raise SummonError(
str(exc) + " at '{}' in '{}'".format(summon_file, repo),
cause=exc.cause
)
def pull_dependencies(deps):
if not deps:
return
outs = [_repo.find_out_by_relpath(dep) for dep in deps]
with _repo.state:
for out in outs:
_repo.cloud.pull(out.get_used_cache())
out.checkout()
path = os.path.join(_repo.root_dir, fname)
obj = _get_object_from_summoners_file(name, path)
info = obj["summon"]
pull_dependencies(info.get("deps"))
_pull_dependencies(_repo, info.get("deps"))
_args = copy.deepcopy(info.get("args", {}))
_args.update(args or {})
@@ -124,26 +117,43 @@ def summon(name, fname="dvcsummon.yaml", args=None, repo=None, rev=None):
return _invoke_method(info["call"], _args, path=_repo.root_dir)
def _get_object_from_summoners_file(name, path):
def _get_object_from_summon_file(name, path):
"""
Given a summonable object's name, search for it on the given file
and bring it to life.
and return its description.
"""
try:
with open(path, "r") as fobj:
content = summon_schema(ruamel.yaml.safe_load(fobj.read()))
objects = content["objects"]
return next(x for x in objects if x["name"] == name)
content = SUMMON_SCHEMA(ruamel.yaml.safe_load(fobj.read()))
objects = [x for x in content["objects"] if x["name"] == name]
if not objects:
raise SummonError("No object with name '{}'".format(name))
elif len(objects) >= 2:
raise SummonError(
"More than one object with name '{}'".format(name)
)
return objects[0]
except FileMissingError:
# XXX: Prints ugly absolute path.
raise SummonError("No such YAML file with path: '{}'".format(path))
raise SummonError("Summon file not found")
except ruamel.yaml.YAMLError as exc:
raise SummonError("Failed to parse YAML correctly", exc)
raise SummonError("Failed to parse summon file", exc)
except Invalid as exc:
# XXX: Doesn't point out the error.
raise SummonError("YAML file dosen't match with the schema", exc)
except StopIteration:
raise SummonError("No such object with name '{}'".format(name))
raise SummonError(str(exc))
def _pull_dependencies(repo, deps):
if not deps:
return
outs = [repo.find_out_by_relpath(dep) for dep in deps]
with repo.state:
for out in outs:
repo.cloud.pull(out.get_used_cache())
out.checkout()
def _invoke_method(call, args, path):
@@ -151,6 +161,9 @@ def _invoke_method(call, args, path):
# * Not thread safe
# * Import will pollute sys.modules
# * Weird errors if there is a name clash within sys.modules
# XXX: sys.path manipulation is "theoretically" not needed
# but tests are failing for an unknown reason.
cwd = os.path.abspath(os.curdir)
try:
@@ -163,23 +176,17 @@ def _invoke_method(call, args, path):
sys.path.pop()
def _import_string(import_name, silent=False):
def _import_string(import_name):
"""Imports an object based on a string.
Useful to delay import to not load everything on startup.
Use dotted notaion in `import_name`, e.g. 'dvc.remote.gs.RemoteGS'.
If the `silent` is True the return value will be `None` if the import
fails.
:return: imported object
"""
import_name = builtin_str(import_name)
try:
if "." in import_name:
module, obj = import_name.rsplit(".", 1)
else:
return importlib.import_module(import_name)
return getattr(importlib.import_module(module), obj)
except (ImportError, AttributeError):
if not silent:
raise
if "." in import_name:
module, obj = import_name.rsplit(".", 1)
else:
return importlib.import_module(import_name)
return getattr(importlib.import_module(module), obj)
+24 -10
View File
@@ -137,7 +137,7 @@ def test_summon(tmp_dir, erepo_dir, dvc, monkeypatch):
"objects": [
{
"name": "sum",
"description": "Add <x> to <number>",
"meta": { "description": "Add <x> to <number>" },
"summon": {
"type": "python",
"call": "calculator.add_to_num",
@@ -151,8 +151,12 @@ def test_summon(tmp_dir, erepo_dir, dvc, monkeypatch):
different_objects = copy.deepcopy(objects)
different_objects["objects"][0]["summon"]["args"]["x"] = 100
dup_objects= copy.deepcopy(objects)
dup_objects["objects"] *= 2
dvcsummon_yaml = ruamel.yaml.dump(objects)
different_yaml = ruamel.yaml.dump(different_objects)
dup_yaml = ruamel.yaml.dump(dup_objects)
invalid_yaml = ruamel.yaml.dump({"name": "sum"})
not_yaml = "a: - this is not a valid YAML file"
@@ -162,6 +166,7 @@ def test_summon(tmp_dir, erepo_dir, dvc, monkeypatch):
erepo_dir.dvc_gen("number", "100", commit="Add number.dvc")
erepo_dir.scm_gen("dvcsummon.yaml", dvcsummon_yaml)
erepo_dir.scm_gen("different.yaml", different_yaml)
erepo_dir.scm_gen("dup.yaml", dup_yaml)
erepo_dir.scm_gen("invalid.yaml", invalid_yaml)
erepo_dir.scm_gen("not_yaml.yaml", not_yaml)
erepo_dir.scm_gen(
@@ -174,16 +179,25 @@ def test_summon(tmp_dir, erepo_dir, dvc, monkeypatch):
assert api.summon("sum", repo=repo_url) == 101
assert api.summon("sum", repo=repo_url, args={"x": 2}) == 102
assert api.summon("sum", repo=repo_url, fname="different.yaml") == 200
assert api.summon("sum", repo=repo_url, summon_file="different.yaml") == 200
with pytest.raises(SummonError):
api.summon("sum", repo=repo_url, fname="not-existent-file.yaml")
try:
api.summon("sum", repo=repo_url, summon_file="missing.yaml")
except SummonError as exc:
assert "Summon file not found" in str(exc)
assert "missing.yaml" in str(exc)
assert repo_url in str(exc)
else:
pytest.fail("Did not raise on missing summon file")
with pytest.raises(SummonError):
api.summon("non-existent", repo=repo_url)
with pytest.raises(SummonError, match=r"No object with name 'missing'"):
api.summon("missing", repo=repo_url)
with pytest.raises(SummonError):
api.summon("sum", repo=repo_url, fname="invalid.yaml")
with pytest.raises(SummonError, match=r"More than one object with name 'sum'"):
api.summon("sum", repo=repo_url, summon_file="dup.yaml")
with pytest.raises(SummonError):
api.summon("sum", repo=repo_url, fname="not_yaml.yaml")
with pytest.raises(SummonError, match=r"extra keys not allowed"):
api.summon("sum", repo=repo_url, summon_file="invalid.yaml")
with pytest.raises(SummonError, match=r"Failed to parse summon file"):
api.summon("sum", repo=repo_url, summon_file="not_yaml.yaml")