Skip to content

Commit

Permalink
Merge pull request #3251 from Suor/summon-publish
Browse files Browse the repository at this point in the history
api: prepare for dvcx summon/publish
  • Loading branch information
efiop authored Jan 30, 2020
2 parents fd1e6f4 + f8c4001 commit d864caf
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 228 deletions.
159 changes: 0 additions & 159 deletions dvc/api.py
Original file line number Diff line number Diff line change
@@ -1,48 +1,11 @@
from builtins import open as builtin_open
import importlib
import os
import sys
from contextlib import contextmanager, _GeneratorContextManager as GCM
import threading

from funcy import wrap_with
import ruamel.yaml
from voluptuous import Schema, Required, Invalid

from dvc.repo import Repo
from dvc.exceptions import DvcException, NotDvcRepoError
from dvc.external_repo import external_repo


SUMMON_FILE_SCHEMA = Schema(
{
Required("objects"): [
{
Required("name"): str,
"meta": dict,
Required("summon"): {
Required("type"): str,
"deps": [str],
str: object,
},
}
]
}
)
SUMMON_PYTHON_SCHEMA = Schema(
{
Required("type"): "python",
Required("call"): str,
"args": dict,
"deps": [str],
}
)


class SummonError(DvcException):
pass


class UrlNotDvcRepoError(DvcException):
"""Thrown if given url is not a DVC repository.
Expand Down Expand Up @@ -120,128 +83,6 @@ def _make_repo(repo_url=None, rev=None):
yield repo


def summon(name, repo=None, rev=None, summon_file="dvcsummon.yaml", args=None):
"""Instantiate an object described in the `summon_file`."""
with prepare_summon(
name, repo=repo, rev=rev, summon_file=summon_file
) as desc:
try:
summon_dict = SUMMON_PYTHON_SCHEMA(desc.obj["summon"])
except Invalid as exc:
raise SummonError(str(exc)) from exc

_args = {**summon_dict.get("args", {}), **(args or {})}
return _invoke_method(summon_dict["call"], _args, desc.repo.root_dir)


@contextmanager
def prepare_summon(name, repo=None, rev=None, summon_file="dvcsummon.yaml"):
"""Does a couple of things every summon needs as a prerequisite:
clones the repo, parses the summon file and pulls the deps.
Calling code is expected to complete the summon logic following
instructions stated in "summon" dict of the object spec.
Returns a SummonDesc instance, which contains references to a Repo object,
named object specification and resolved paths to deps.
"""
with _make_repo(repo, rev=rev) as _repo:
_require_dvc(_repo)
try:
path = os.path.join(_repo.root_dir, summon_file)
obj = _get_object_spec(name, path)
yield SummonDesc(_repo, obj)
except SummonError as exc:
raise SummonError(
str(exc) + " at '{}' in '{}'".format(summon_file, repo)
) from exc.__cause__


class SummonDesc:
def __init__(self, repo, obj):
self.repo = repo
self.obj = obj
self._pull_deps()

@property
def deps(self):
return [os.path.join(self.repo.root_dir, d) for d in self._deps]

@property
def _deps(self):
return self.obj["summon"].get("deps", [])

def _pull_deps(self):
if not self._deps:
return

outs = [self.repo.find_out_by_relpath(d) for d in self._deps]

with self.repo.state:
for out in outs:
self.repo.cloud.pull(out.get_used_cache())
out.checkout()


def _get_object_spec(name, path):
"""
Given a summonable object's name, search for it on the given file
and return its description.
"""
try:
with builtin_open(path, "r") as fobj:
content = SUMMON_FILE_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 FileNotFoundError as exc:
raise SummonError("Summon file not found") from exc
except ruamel.yaml.YAMLError as exc:
raise SummonError("Failed to parse summon file") from exc
except Invalid as exc:
raise SummonError(str(exc)) from exc


@wrap_with(threading.Lock())
def _invoke_method(call, args, path):
# XXX: Some issues with this approach:
# * Import will pollute sys.modules
# * sys.path manipulation is "theoretically" not needed,
# but tests are failing for an unknown reason.
cwd = os.getcwd()

try:
os.chdir(path)
sys.path.insert(0, path)
method = _import_string(call)
return method(**args)
finally:
os.chdir(cwd)
sys.path.pop(0)


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'.
:return: imported object
"""
if "." in import_name:
module, obj = import_name.rsplit(".", 1)
else:
return importlib.import_module(import_name)
return getattr(importlib.import_module(module), obj)


def _require_dvc(repo):
if not isinstance(repo, Repo):
raise UrlNotDvcRepoError(repo.url)
2 changes: 1 addition & 1 deletion dvc/repo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def tree(self, tree):
self._reset()

def __repr__(self):
return "Repo: '{root_dir}'".format(root_dir=self.root_dir)
return "{}: '{}'".format(self.__class__.__name__, self.root_dir)

@classmethod
def find_root(cls, root=None):
Expand Down
10 changes: 10 additions & 0 deletions dvc/scm/git/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,16 @@ def checkout(self, branch, create_new=False):
else:
self.repo.git.checkout(branch)

def pull(self):
info, = self.repo.remote().pull()
if info.flags & info.ERROR:
raise SCMError("pull failed: {}".format(info.note))

def push(self):
info, = self.repo.remote().push()
if info.flags & info.ERROR:
raise SCMError("push failed: {}".format(info.summary))

def branch(self, branch):
self.repo.git.branch(branch)

Expand Down
69 changes: 1 addition & 68 deletions tests/func/test_api.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import os
import shutil
import copy

import ruamel.yaml
import pytest

from dvc import api
from dvc.api import SummonError, UrlNotDvcRepoError
from dvc.api import UrlNotDvcRepoError
from dvc.compat import fspath
from dvc.exceptions import FileMissingError
from dvc.main import main
Expand Down Expand Up @@ -141,68 +139,3 @@ def test_open_not_cached(dvc):
os.remove(metric_file)
with pytest.raises(FileMissingError):
api.read(metric_file)


def test_summon(tmp_dir, dvc, erepo_dir):
objects = {
"objects": [
{
"name": "sum",
"meta": {"description": "Add <x> to <number>"},
"summon": {
"type": "python",
"call": "calculator.add_to_num",
"args": {"x": 1},
"deps": ["number"],
},
}
]
}

other_objects = copy.deepcopy(objects)
other_objects["objects"][0]["summon"]["args"]["x"] = 100

dup_objects = copy.deepcopy(objects)
dup_objects["objects"] *= 2

with erepo_dir.chdir():
erepo_dir.dvc_gen("number", "100", commit="Add number.dvc")
erepo_dir.scm_gen("dvcsummon.yaml", ruamel.yaml.dump(objects))
erepo_dir.scm_gen("other.yaml", ruamel.yaml.dump(other_objects))
erepo_dir.scm_gen("dup.yaml", ruamel.yaml.dump(dup_objects))
erepo_dir.scm_gen("invalid.yaml", ruamel.yaml.dump({"name": "sum"}))
erepo_dir.scm_gen("not_yaml.yaml", "a: - this is not a YAML file")
erepo_dir.scm_gen(
"calculator.py",
"def add_to_num(x): return x + int(open('number').read())",
)
erepo_dir.scm.commit("Add files")

repo_url = "file://{}".format(erepo_dir)

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, summon_file="other.yaml") == 200

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, match=r"No object with name 'missing'"):
api.summon("missing", repo=repo_url)

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, 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")

0 comments on commit d864caf

Please sign in to comment.