From b3b4daf7fa39a7c0c5e1282d875ccb9d7cba2e07 Mon Sep 17 00:00:00 2001
From: Dmitry <dmitry.petrov@nevesomo.com>
Date: Sun, 3 Sep 2017 19:01:05 -0700
Subject: [PATCH] Refactoring: make methods smaller

---
 dvc/git_wrapper.py | 47 +++++++++++++++++++++++++++-------------------
 dvc/workflow.py    | 11 +++++++----
 2 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py
index 5b93c9f07..1dc896a50 100644
--- a/dvc/git_wrapper.py
+++ b/dvc/git_wrapper.py
@@ -260,29 +260,38 @@ class GitWrapper(GitWrapperI):
         files = set(Executor.exec_cmd_only_success(git_cmd).split('\n'))
 
         if target in files:
-            try:
-                settings.path_factory.data_item(target)
-            except DataItemError as ex:
-                Logger.warn('Target file {} is not data item: {}'.format(target, ex))
-                return True, None
+            symlink_content = self._get_symlink_content(hash, target, settings)
+            if symlink_content is not None:
+                metric = self.target_metric_from_git_history(hash, symlink_content, target, settings)
+            else:
+                metric = None
 
-            try:
-                cmd_symlink_data = ['git', 'show', '{}:{}'.format(hash, target)]
-                symlink_content = Executor.exec_cmd_only_success(cmd_symlink_data).split('\n')
-            except ExecutorError as ex:
-                msg = '[dvc-git] Cannot obtain content of target symbolic file {} with hash {}: {}'
-                Logger.warn(msg.format(target, hash, ex))
-                return True, None
-
-            if not symlink_content or len(symlink_content) != 1:
-                msg = '[dvc-git] Target symbolic file {} with hash {} has wrong format'
-                Logger.warn(msg.format(target, hash))
-                return True, None
-
-            return True, self.target_metric_from_git_history(hash, symlink_content[0], target, settings)
+            return True, metric
 
         return False, None
 
+    def _get_symlink_content(self, hash, target, settings):
+        try:
+            settings.path_factory.data_item(target)
+        except DataItemError as ex:
+            Logger.warn('Target file {} is not data item: {}'.format(target, ex))
+            return None
+
+        try:
+            cmd_symlink_data = ['git', 'show', '{}:{}'.format(hash, target)]
+            symlink_content = Executor.exec_cmd_only_success(cmd_symlink_data).split('\n')
+        except ExecutorError as ex:
+            msg = '[dvc-git] Cannot obtain content of target symbolic file {} with hash {}: {}'
+            Logger.warn(msg.format(target, hash, ex))
+            return None
+
+        if not symlink_content or len(symlink_content) != 1:
+            msg = '[dvc-git] Target symbolic file {} with hash {} has wrong format'
+            Logger.warn(msg.format(target, hash))
+            return None
+
+        return symlink_content[0]
+
     def target_metric_from_git_history(self, hash, symlink_content, target, settings):
         cache_rel_to_data = os.path.relpath(settings.config.cache_dir, settings.config.data_dir)
         common_prefix = os.path.commonprefix([symlink_content, cache_rel_to_data])
diff --git a/dvc/workflow.py b/dvc/workflow.py
index bb3cb7548..e60c356ec 100644
--- a/dvc/workflow.py
+++ b/dvc/workflow.py
@@ -152,10 +152,7 @@ class Workflow(object):
         child_commit_hashes = self._edges.get(commit.hash)
 
         if child_commit_hashes and all(self._commits[hash].is_repro for hash in child_commit_hashes):
-            if commit.has_target_metric:
-                for hash in child_commit_hashes:
-                    if not self._commits[hash].has_target_metric:
-                        self._commits[hash].set_target_metric(commit.target_metric)
+            self._upstream_target_metric(child_commit_hashes, commit)
 
             for hash in child_commit_hashes:
                 self._commits[hash].add_parents(commit.parent_hashes)
@@ -170,6 +167,12 @@ class Workflow(object):
             return False
         pass
 
+    def _upstream_target_metric(self, child_commit_hashes, commit):
+        if commit.has_target_metric:
+            for hash in child_commit_hashes:
+                if not self._commits[hash].has_target_metric:
+                    self._commits[hash].set_target_metric(commit.target_metric)
+
     @staticmethod
     def _redirect_edges(edges, child_commit_hashes, hash, commit_hashes):
         for h in commit_hashes: