From aaa9167a00acd7cb37c6b63992116196c8176ddc Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 28 Mar 2019 00:17:19 +0200 Subject: [PATCH] dvc: fix remote.checkout() to follow same conventions Currently `checkout()` accepts `OutputBase*` class as an argument, which doesn't follow conventions from all other methods, which accept path_info and/or checksum_info. To fix that, cache for dir info is moved from output to the underlying remote. Signed-off-by: Ruslan Kuprieiev --- dvc/output/base.py | 2 +- dvc/output/local.py | 7 +------ dvc/remote/base.py | 32 ++++++++++++++++++-------------- dvc/remote/local.py | 24 +++++++++++++++--------- 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/dvc/output/base.py b/dvc/output/base.py index 1aa7e1488..ff3d8de24 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -212,7 +212,7 @@ class OutputBase(object): return getattr(self.repo.cache, self.scheme).checkout( - output=self, force=force, progress_callback=progress_callback + self.path_info, self.info, force=force, progress_callback=progress_callback ) def remove(self, ignore_remove=False): diff --git a/dvc/output/local.py b/dvc/output/local.py index 70728eb04..6a76cdd8e 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -155,12 +155,7 @@ class OutputLOCAL(OutputBase): @property def dir_cache(self): - if self.checksum not in self._dir_cache.keys(): - self._dir_cache[ - self.checksum - ] = self.repo.cache.local.load_dir_cache(self.checksum) - - return self._dir_cache[self.checksum] + return self.repo.cache.local.load_dir_cache(self.checksum) def get_files_number(self): if self.cache is None: diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 714421968..cdba4c620 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -322,10 +322,9 @@ class RemoteBase(object): self.remove(path_info) - def do_checkout(self, output, force=False, progress_callback=None): - path_info = output.path_info - checksum = output.info.get(self.PARAM_CHECKSUM) - + def do_checkout( + self, path_info, checksum, force=False, progress_callback=None + ): if self.exists(path_info): msg = "data '{}' exists. Removing before checkout." logger.warning(msg.format(str(path_info))) @@ -334,33 +333,38 @@ class RemoteBase(object): from_info = self.checksum_to_path_info(checksum) self.copy(from_info, path_info) - def checkout(self, output, force=False, progress_callback=None): - scheme = output.path_info["scheme"] + def checkout( + self, path_info, checksum_info, force=False, progress_callback=None + ): + scheme = path_info["scheme"] if scheme not in ["", "local"] and scheme != self.scheme: raise NotImplementedError - checksum = output.info.get(self.PARAM_CHECKSUM) + checksum = checksum_info.get(self.PARAM_CHECKSUM) if not checksum: msg = "No checksum info for '{}'." - logger.debug(msg.format(str(output.path_info))) + logger.debug(msg.format(str(path_info))) return - if not self.changed(output.path_info, output.info): + if not self.changed(path_info, checksum_info): msg = "Data '{}' didn't change." - logger.debug(msg.format(str(output.path_info))) + logger.debug(msg.format(str(path_info))) return if self.changed_cache(checksum): msg = "Cache '{}' not found. File '{}' won't be created." - logger.warning(msg.format(checksum, str(output.path_info))) - self.safe_remove(output.path_info, force=force) + logger.warning(msg.format(checksum, str(path_info))) + self.safe_remove(path_info, force=force) return msg = "Checking out '{}' with cache '{}'." - logger.debug(msg.format(str(output.path_info), checksum)) + logger.debug(msg.format(str(path_info), checksum)) self.do_checkout( - output, force=force, progress_callback=progress_callback + path_info, + checksum, + force=force, + progress_callback=progress_callback, ) @staticmethod diff --git a/dvc/remote/local.py b/dvc/remote/local.py index b59e64109..702f34ad2 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -78,6 +78,8 @@ class RemoteLOCAL(RemoteBase): self.path_info = {"scheme": "local"} + self._dir_info = {} + @staticmethod def compat_config(config): ret = config.copy() @@ -256,9 +258,12 @@ class RemoteLOCAL(RemoteBase): def load_dir_cache(self, md5): path = self.get(md5) - assert self.is_dir_cache(path) + dir_info = self._dir_info.get(md5) + if dir_info: + return dir_info + try: with open(path, "r") as fd: d = json.load(fd) @@ -275,6 +280,8 @@ class RemoteLOCAL(RemoteBase): for info in d: info["relpath"] = self.to_ospath(info["relpath"]) + self._dir_info[md5] = d + return d def dump_dir_cache(self, md5, dir_info): @@ -299,15 +306,13 @@ class RemoteLOCAL(RemoteBase): def is_dir_cache(cls, cache): return cache.endswith(cls.MD5_DIR_SUFFIX) - def do_checkout(self, output, force=False, progress_callback=None): - path_info = output.path_info - checksum = output.info.get(self.PARAM_CHECKSUM) - + def do_checkout( + self, path_info, checksum, force=False, progress_callback=None + ): path = path_info["path"] - md5 = checksum - cache = self.get(md5) + cache = self.get(checksum) - if not output.is_dir_cache: + if not self.is_dir_cache(cache): if os.path.exists(path): self.safe_remove(path_info, force=force) @@ -322,11 +327,12 @@ class RemoteLOCAL(RemoteBase): if not os.path.exists(path): os.makedirs(path) - dir_info = output.dir_cache dir_relpath = os.path.relpath(path) logger.debug("Linking directory '{}'.".format(dir_relpath)) + dir_info = self.load_dir_cache(checksum) + for processed, entry in enumerate(dir_info): relpath = entry[self.PARAM_RELPATH] m = entry[self.PARAM_CHECKSUM]