checkout: raise on non existing dir cache (#2514)

* checkout: raise on non existing dir cache

* checkout: don't throw on failed load_dir_cache

* checkout: raise on failed checkouts, only after all checkouts are perfromed

* remote: base: raise on failed dir cache loading

* rename exception for failed checkout

dvc/exceptions.py

@@ -321,3 +321,13 @@ def __init__(self, amount):
         super(UploadError, self).__init__(
             "{amount} files failed to upload".format(amount=amount)
         )
+
+
+class CheckoutError(DvcException):
+    def __init__(self, target_infos):
+        targets = [str(t) for t in target_infos]
+        m = (
+            "Checkout failed for following targets:\n {}\nDid you "
+            "forget to fetch?".format("\n".join(targets))
+        )
+        super(CheckoutError, self).__init__(m)

dvc/output/base.py

@@ -294,7 +294,7 @@ def checkout(self, force=False, progress_callback=None, tag=None):
         else:
             info = self.info
 
-        self.cache.checkout(
+        return self.cache.checkout(
             self.path_info,
             info,
             force=force,

dvc/remote/base.py

@@ -71,6 +71,14 @@ class RemoteMissingDepsError(DvcException):
     pass
 
 
+class DirCacheError(DvcException):
+    def __init__(self, checksum, cause=None):
+        super(DirCacheError, self).__init__(
+            "Failed to load dir cache for checksum: '{}'.".format(checksum),
+            cause=cause,
+        )
+
+
 class RemoteBASE(object):
     scheme = "base"
     path_cls = URLInfo
@@ -256,7 +264,11 @@ def get_dir_cache(self, checksum):
         if dir_info:
             return dir_info
 
-        dir_info = self.load_dir_cache(checksum)
+        try:
+            dir_info = self.load_dir_cache(checksum)
+        except DirCacheError:
+            dir_info = []
+
         self._dir_info[checksum] = dir_info
         return dir_info
 
@@ -266,9 +278,8 @@ def load_dir_cache(self, checksum):
         try:
             with self.cache.open(path_info, "r") as fobj:
                 d = json.load(fobj)
-        except (ValueError, FileNotFoundError):
-            logger.exception("Failed to load dir cache '{}'".format(path_info))
-            return []
+        except (ValueError, FileNotFoundError) as exc:
+            raise DirCacheError(checksum, cause=exc)
 
         if not isinstance(d, list):
             msg = "dir cache file format error '{}' [skipping the file]"
@@ -848,32 +859,33 @@ def checkout(
             raise NotImplementedError
 
         checksum = checksum_info.get(self.PARAM_CHECKSUM)
-        skip = False
+        failed = None
         if not checksum:
             logger.warning(
                 "No checksum info found for '{}'. "
                 "It won't be created.".format(str(path_info))
             )
             self.safe_remove(path_info, force=force)
-            skip = True
+            failed = path_info
 
         elif self.changed_cache(checksum):
             msg = "Cache '{}' not found. File '{}' won't be created."
             logger.warning(msg.format(checksum, str(path_info)))
             self.safe_remove(path_info, force=force)
-            skip = True
+            failed = path_info
 
-        if skip:
+        if failed:
             if progress_callback:
                 progress_callback(
                     str(path_info), self.get_files_number(checksum)
                 )
-            return
+            return failed
 
         msg = "Checking out '{}' with cache '{}'."
         logger.debug(msg.format(str(path_info), checksum))
 
         self._checkout(path_info, checksum, force, progress_callback)
+        return None
 
     def _checkout(
         self, path_info, checksum, force=False, progress_callback=None

dvc/repo/checkout.py

@@ -2,7 +2,7 @@
 
 import logging
 
-from dvc.exceptions import CheckoutErrorSuggestGit
+from dvc.exceptions import CheckoutErrorSuggestGit, CheckoutError
 from dvc.progress import Tqdm
 
 
@@ -42,6 +42,7 @@ def _checkout(
         total = get_all_files_numbers(stages)
         if total == 0:
             logger.info("Nothing to do")
+        failed = []
         with Tqdm(
             total=total, unit="file", desc="Checkout", disable=total == 0
         ) as pbar:
@@ -54,4 +55,10 @@ def _checkout(
                         )
                     )
 
-                stage.checkout(force=force, progress_callback=pbar.update_desc)
+                failed.extend(
+                    stage.checkout(
+                        force=force, progress_callback=pbar.update_desc
+                    )
+                )
+        if failed:
+            raise CheckoutError(failed)

dvc/stage.py

@@ -872,10 +872,14 @@ def check_missing_outputs(self):
             raise MissingDataSource(paths)
 
     def checkout(self, force=False, progress_callback=None):
+        failed_checkouts = []
         for out in self.outs:
-            out.checkout(
+            failed = out.checkout(
                 force=force, tag=self.tag, progress_callback=progress_callback
             )
+            if failed:
+                failed_checkouts.append(failed)
+        return failed_checkouts
 
     @staticmethod
     def _status(entries):

tests/func/test_cache.py

@@ -6,6 +6,7 @@
 
 from dvc.cache import Cache
 from dvc.main import main
+from dvc.remote.base import DirCacheError
 from dvc.utils import relpath
 
 from tests.basic_env import TestDvc, TestDir
@@ -49,7 +50,8 @@ def test(self):
         checksum = "123.dir"
         fname = self.dvc.cache.local.get(checksum)
         self.create(fname, "<clearly>not,json")
-        self._do_test(self.dvc.cache.local.load_dir_cache(checksum))
+        with pytest.raises(DirCacheError):
+            self.dvc.cache.local.load_dir_cache(checksum)
 
         checksum = "234.dir"
         fname = self.dvc.cache.local.get(checksum)

tests/func/test_checkout.py

@@ -21,6 +21,7 @@
     ConfirmRemoveError,
     TargetNotDirectoryError,
     CheckoutErrorSuggestGit,
+    CheckoutError,
 )
 
 from mock import patch
@@ -74,7 +75,8 @@ def test(self):
         with open(cache, "a") as fd:
             fd.write("1")
 
-        self.dvc.checkout(force=True)
+        with pytest.raises(CheckoutError):
+            self.dvc.checkout(force=True)
 
         self.assertFalse(os.path.isfile(self.FOO))
         self.assertFalse(os.path.isfile(cache))
@@ -101,7 +103,8 @@ def test(self):
         with open(cache, "w+") as fobj:
             fobj.write("1")
 
-        self.dvc.checkout(force=True)
+        with pytest.raises(CheckoutError):
+            self.dvc.checkout(force=True)
 
         self.assertFalse(os.path.exists(cache))
 
@@ -302,7 +305,8 @@ def test(self):
         del d[Stage.PARAM_DEPS][0][RemoteLOCAL.PARAM_CHECKSUM]
         dump_stage_file(self.file1_stage, d)
 
-        self.dvc.checkout(force=True)
+        with pytest.raises(CheckoutError):
+            self.dvc.checkout(force=True)
 
 
 class TestCheckoutEmptyDir(TestDvc):
@@ -476,7 +480,8 @@ def test(self):
 
 def test_checkout_no_checksum(repo_dir, dvc_repo):
     stage = dvc_repo.run(outs=[repo_dir.FOO], no_exec=True, cmd="somecmd")
-    dvc_repo.checkout(stage.path, force=True)
+    with pytest.raises(CheckoutError):
+        dvc_repo.checkout(stage.path, force=True)
     assert not os.path.exists(repo_dir.FOO)