Skip to content

Commit

Permalink
run: try to save deps before running the command (#3709)
Browse files Browse the repository at this point in the history
Unlike old `_check_missing_deps`, this also verifies that we are able to
save more complex dependencies such as parameters, where we not only
care about the config file, but also about the parameters in it.

Fixes #3707
  • Loading branch information
efiop authored Apr 30, 2020
1 parent d998c0a commit 71d156a
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 23 deletions.
18 changes: 6 additions & 12 deletions dvc/stage/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
StagePathNotDirectoryError,
StageCommitError,
StageUpdateError,
MissingDep,
MissingDataSource,
)
from . import params
Expand Down Expand Up @@ -374,8 +373,7 @@ def is_cached(self):

# NOTE: need to save checksums for deps in order to compare them
# with what is written in the old stage.
for dep in self.deps:
dep.save()
self._save_deps()

old_d = old.dumpd()
new_d = self.dumpd()
Expand Down Expand Up @@ -482,10 +480,13 @@ def _compute_md5(self):
logger.debug("Computed {} md5: '{}'".format(self, m))
return m

def save(self):
def _save_deps(self):
for dep in self.deps:
dep.save()

def save(self):
self._save_deps()

for out in self.outs:
out.save()

Expand Down Expand Up @@ -526,12 +527,6 @@ def commit(self):
for out in self.outs:
out.commit()

def _check_missing_deps(self):
missing = [dep for dep in self.deps if not dep.exists]

if any(missing):
raise MissingDep(missing)

@staticmethod
def _warn_if_fish(executable): # pragma: no cover
if (
Expand Down Expand Up @@ -570,8 +565,6 @@ def _check_duplicated_arguments(self):

@unlocked_repo
def _run(self):
self._check_missing_deps()

kwargs = {"cwd": self.wdir, "env": fix_env(None), "close_fds": True}

if os.name == "nt":
Expand Down Expand Up @@ -667,6 +660,7 @@ def run(
logger.info("Stage is cached, skipping.")
self.checkout()
else:
self._save_deps()
logger.info("Running command:\n\t{}".format(self.cmd))
self._run()

Expand Down
9 changes: 0 additions & 9 deletions dvc/stage/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,6 @@ def __init__(self, path):
)


class MissingDep(DvcException):
def __init__(self, deps):
assert len(deps) > 0

dep = "dependencies" if len(deps) > 1 else "dependency"
msg = "missing '{}': {}".format(dep, ", ".join(map(str, deps)))
super().__init__(msg)


class MissingDataSource(DvcException):
def __init__(self, missing_files):
assert len(missing_files) > 0
Expand Down
5 changes: 3 additions & 2 deletions tests/func/test_run_single_stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
StagePathOutsideError,
StagePathNotFoundError,
StagePathNotDirectoryError,
MissingDep,
)
from dvc.system import System
from dvc.utils import file_md5
Expand Down Expand Up @@ -79,7 +78,9 @@ def test(self):

class TestRunMissingDep(TestDvc):
def test(self):
with self.assertRaises(MissingDep):
from dvc.dependency.base import DependencyDoesNotExistError

with self.assertRaises(DependencyDoesNotExistError):
self.dvc.run(
cmd="",
deps=["non-existing-dep"],
Expand Down

0 comments on commit 71d156a

Please sign in to comment.