metrics: check that specified path is a text file and not a directory or binary file

Fixes #911

Signed-off-by: Ruslan Kuprieiev <[email protected]>

dvc/output/local.py

@@ -8,6 +8,7 @@ from dvc.dependency.local import DependencyLOCAL
 from dvc.exceptions import DvcException
 from dvc.logger import Logger
 from dvc.utils import remove
+from dvc.istextfile import istextfile
 
 
 class OutputLOCAL(DependencyLOCAL):
@@ -57,9 +58,22 @@ class OutputLOCAL(DependencyLOCAL):
 
         self.project.cache.local.checkout(self.path_info, self.info)
 
+    def _verify_metric(self):
+        if not self.metric:
+            return
+
+        if os.path.isdir(self.path):
+            msg = 'Directory \'{}\' cannot be used as metrics.'
+            raise DvcException(msg.format(self.rel_path))
+
+        if not istextfile(self.path):
+            msg = 'Binary file \'{}\' cannot be used as metrics.'
+            raise DvcException(msg.format(self.rel_path))
+
     def save(self):
         if not self.use_cache:
             super(OutputLOCAL, self).save()
+            self._verify_metric()
             msg = 'Output \'{}\' doesn\'t use cache. Skipping saving.'
             self.project.logger.debug(msg.format(self.rel_path))
             return

dvc/project.py

@@ -506,6 +506,7 @@ class Project(object):
                     raise DvcException(msg.format(out.rel_path))
 
                 out.metric = val
+                out._verify_metric()
 
             stage.dump()
 

tests/test_metrics.py

@@ -2,6 +2,7 @@ import os
 import json
 
 from dvc.main import main
+from dvc.exceptions import DvcException
 from tests.basic_env import TestDvc
 
 
@@ -78,13 +79,9 @@ class TestMetrics(TestDvc):
 
 class TestMetricsReproCLI(TestDvc):
     def test(self):
-
-        stage = self.dvc.run(outs_no_cache=['metrics'],
+        stage = self.dvc.run(metrics_no_cache=['metrics'],
                              cmd='python {} {} {}'.format(self.CODE, self.FOO, 'metrics'))
 
-        ret = main(['metrics', 'add', 'metrics'])
-        self.assertEqual(ret, 0)
-
         ret = main(['repro', '-m', stage.path])
         self.assertEqual(ret, 0)
 
@@ -94,6 +91,19 @@ class TestMetricsReproCLI(TestDvc):
         ret = main(['repro', '-f', '-m', stage.path])
         self.assertEqual(ret, 0)
 
+    def test_dir(self):
+        os.mkdir('metrics_dir')
+
+        with self.assertRaises(DvcException):
+            self.dvc.run(metrics_no_cache=['metrics_dir'])
+
+    def test_binary(self):
+        with open('metrics_bin', 'w+') as fd:
+            fd.write('\0\0\0\0\0\0\0\0')
+
+        with self.assertRaises(DvcException):
+            self.dvc.run(metrics_no_cache=['metrics_bin'])
+
 
 class TestMetricsCLI(TestMetrics):
     def test(self):
@@ -115,3 +125,18 @@ class TestMetricsCLI(TestMetrics):
 
         ret = main(['metrics', 'show', '--all-branches', 'metric_hcsv', '--hcsv-path', 'branch,0'])
         self.assertEqual(ret, 0)
+
+    def test_dir(self):
+        os.mkdir('metrics_dir')
+
+        with self.assertRaises(DvcException):
+            self.dvc.run(outs_no_cache=['metrics_dir'])
+            self.dvc.metrics_add('metrics_dir')
+
+    def test_binary(self):
+        with open('metrics_bin', 'w+') as fd:
+            fd.write('\0\0\0\0\0\0\0\0')
+
+        with self.assertRaises(DvcException):
+            self.dvc.run(outs_no_cache=['metrics_bin'])
+            self.dvc.metrics_add('metrics_bin')