From d2f8cb052000fcc978ba208c5889fa6280ebbd61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Tue, 8 Oct 2019 14:23:42 +0200 Subject: [PATCH] api: open scm controlled files (#2566) * tree: make open method use mode and encoding * api: try to open using self.tree if output not found * repo: rename OutputMissingException * test: api: test adding not cached output * api: open: exists test instead of try * repo: open: try opening from tree first * repo: open: handle missing and not cached files * repo: open: separate find out logic from opening --- dvc/exceptions.py | 9 +++++---- dvc/repo/__init__.py | 29 ++++++++++++++++++++++------- dvc/scm/git/tree.py | 7 ++++--- dvc/scm/tree.py | 8 +++----- tests/func/test_api.py | 33 +++++++++++++++++++++++++++++++-- 5 files changed, 65 insertions(+), 21 deletions(-) diff --git a/dvc/exceptions.py b/dvc/exceptions.py index a352ada15..bca505c1a 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -259,10 +259,11 @@ class ETagMismatchError(DvcException): ) -class OutputFileMissingError(DvcException): - def __init__(self, path): - super(OutputFileMissingError, self).__init__( - "Can't find {} neither locally nor on remote".format(path) +class FileMissingError(DvcException): + def __init__(self, path, cause=None): + super(FileMissingError, self).__init__( + "Can't find '{}' neither locally nor on remote".format(path), + cause=cause, ) diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index fed493531..060f32cff 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -12,7 +12,7 @@ from dvc.config import Config from dvc.exceptions import ( NotDvcRepoError, OutputNotFoundError, - OutputFileMissingError, + FileMissingError, ) from dvc.ignore import DvcIgnoreFilter from dvc.path_info import PathInfo @@ -454,14 +454,29 @@ class Repo(object): @contextmanager def open(self, path, remote=None, mode="r", encoding=None): """Opens a specified resource as a file descriptor""" + cause = None try: - with self._open(path, remote, mode, encoding) as fd: - yield fd - except FileNotFoundError: - raise OutputFileMissingError(relpath(path, self.root_dir)) + out, = self.find_outs_by_path(path) + except OutputNotFoundError as e: + out = None + cause = e - def _open(self, path, remote=None, mode="r", encoding=None): - out, = self.find_outs_by_path(path) + if out and out.use_cache: + try: + with self._open_cached(out, remote, mode, encoding) as fd: + yield fd + return + except FileNotFoundError as e: + raise FileMissingError(relpath(path, self.root_dir), cause=e) + + if self.tree.exists(path): + with self.tree.open(path, mode, encoding) as fd: + yield fd + return + + raise FileMissingError(relpath(path, self.root_dir), cause=cause) + + def _open_cached(self, out, remote=None, mode="r", encoding=None): if out.isdir(): raise ValueError("Can't open a dir") diff --git a/dvc/scm/git/tree.py b/dvc/scm/git/tree.py index 1493f4030..23653b251 100644 --- a/dvc/scm/git/tree.py +++ b/dvc/scm/git/tree.py @@ -45,7 +45,8 @@ class GitTree(BaseTree): def tree_root(self): return self.git.working_dir - def open(self, path, binary=False): + def open(self, path, mode="r", encoding="utf-8"): + assert mode in {"r", "rb"} relative_path = relpath(path, self.git.working_dir) @@ -61,9 +62,9 @@ class GitTree(BaseTree): # the `open()` behavior (since data_stream.read() returns bytes, # and `open` with default "r" mode returns str) data = obj.data_stream.read() - if binary: + if mode == "rb": return BytesIO(data) - return StringIO(data.decode("utf-8")) + return StringIO(data.decode(encoding)) def exists(self, path): return self.git_object_by_path(path) is not None diff --git a/dvc/scm/tree.py b/dvc/scm/tree.py index a90306f76..0931a59d0 100644 --- a/dvc/scm/tree.py +++ b/dvc/scm/tree.py @@ -11,7 +11,7 @@ class BaseTree(object): def tree_root(self): pass - def open(self, path, binary=False): + def open(self, path, mode="r", encoding="utf-8"): """Open file and return a stream.""" def exists(self, path): @@ -42,11 +42,9 @@ class WorkingTree(BaseTree): def tree_root(self): return self.repo_root - def open(self, path, binary=False): + def open(self, path, mode="r", encoding="utf-8"): """Open file and return a stream.""" - if binary: - return open(path, "rb") - return open(path, encoding="utf-8") + return open(path, mode=mode, encoding=encoding) def exists(self, path): """Test whether a path exists.""" diff --git a/tests/func/test_api.py b/tests/func/test_api.py index 2bba711b1..70e36c3ec 100644 --- a/tests/func/test_api.py +++ b/tests/func/test_api.py @@ -1,8 +1,10 @@ +import os + import pytest import shutil from dvc import api -from dvc.exceptions import OutputFileMissingError +from dvc.exceptions import FileMissingError from dvc.main import main from dvc.path_info import URLInfo from dvc.remote.config import RemoteConfig @@ -134,7 +136,7 @@ def test_missing(repo_dir, dvc_repo, remote_url): # Remove cache to make foo missing shutil.rmtree(dvc_repo.cache.local.cache_dir) - with pytest.raises(OutputFileMissingError): + with pytest.raises(FileMissingError): api.read(repo_dir.FOO) @@ -143,3 +145,30 @@ def _set_remote_url_and_commit(repo, remote_url): rconfig.modify("upstream", "url", remote_url) repo.scm.add([repo.config.config_file]) repo.scm.commit("modify remote") + + +def test_open_scm_controlled(dvc_repo, repo_dir): + stage, = dvc_repo.add(repo_dir.FOO) + + stage_content = open(stage.path, "r").read() + with api.open(stage.path) as fd: + assert fd.read() == stage_content + + +def test_open_not_cached(dvc_repo): + metric_file = "metric.txt" + metric_content = "0.6" + metric_code = "open('{}', 'w').write('{}')".format( + metric_file, metric_content + ) + dvc_repo.run( + metrics_no_cache=[metric_file], + cmd=('python -c "{}"'.format(metric_code)), + ) + + with api.open(metric_file) as fd: + assert fd.read() == metric_content + + os.remove(metric_file) + with pytest.raises(FileMissingError): + api.read(metric_file)