dvc: use Popen directly instead of Executor

Executor is no longer useful.

Fixes #434

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

dvc/executor.py

@@ -1,62 +0,0 @@
-"""
-Exec wrapper.
-"""
-import subprocess
-
-from dvc.exceptions import DvcException
-
-
-class ExecutorError(DvcException):
-    """ Executor error exception """
-    def __init__(self, msg):
-        DvcException.__init__(self, msg)
-
-
-class Executor(object):
-    """ Exec wrapper """
-    @staticmethod
-    def exec_cmd(cmd, stdout_file=None, stderr_file=None, cwd=None, shell=False):
-        """ Execute command """
-        stdout, stdout_fd = Executor.output_file(stdout_file)
-        stderr, stderr_fd = Executor.output_file(stderr_file)
-
-        try:
-            proc = subprocess.Popen(cmd,
-                                    cwd=cwd,
-                                    stdout=stdout,
-                                    stderr=stderr,
-                                    shell=shell)
-            proc.wait()
-            out, err = map(lambda s: s.decode().strip('\n\r') if s else '', proc.communicate())
-
-            return proc.returncode, out, err
-        except Exception as ex:
-            return 1, None, str(ex)
-        finally:
-            if stderr_fd:
-                stderr_fd.close()
-            if stdout_fd:
-                stdout_fd.close()
-
-    @staticmethod
-    def output_file(output_file, default_output=None):
-        """ Open output file if specified """
-        output_fd = None
-        if output_file is not None:
-            if output_file == '-':
-                output = default_output
-            else:
-                output_fd = open(output_file, 'w')
-                output = output_fd
-        else:
-            output = subprocess.PIPE
-        return output, output_fd
-
-    @staticmethod
-    def exec_cmd_only_success(cmd, stdout_file=None, stderr_file=None, cwd=None, shell=False):
-        """ Execute command and raise exception if unsuccessfull """
-        code, out, err = Executor.exec_cmd(cmd, stdout_file=stdout_file,
-                                           stderr_file=stderr_file, cwd=cwd, shell=shell)
-        if code != 0:
-            raise ExecutorError('Exec command error:\n{}'.format(err))
-        return out

dvc/stage.py

@@ -2,11 +2,11 @@ import os
 import stat
 import yaml
 import itertools
+import subprocess
 
 from dvc.system import System
 from dvc.utils import file_md5
 from dvc.exceptions import DvcException
-from dvc.executor import Executor
 
 
 class OutputError(DvcException):
@@ -245,6 +245,12 @@ class Output(Dependency):
             os.chmod(self.cache, stat.S_IREAD)
 
 
+class StageCmdFailedError(DvcException):
+    def __init__(self, stage):
+        msg = 'Stage {} cmd {} failed'.format(stage.path, stage.cmd)
+        super(StageCmdFailedError, self).__init__(msg)
+
+
 class Stage(object):
     STAGE_FILE = 'Dvcfile'
     STAGE_FILE_SUFFIX = '.dvc'
@@ -381,7 +387,11 @@ class Stage(object):
         if not self.is_data_source:
             self.project.logger.info(u'Reproducing \'{}\':\n\t{}'.format(self.relpath, self.cmd))
 
-            Executor.exec_cmd_only_success(self.cmd, cwd=str(self.cwd), shell=True)
+            p = subprocess.Popen(self.cmd, cwd=self.cwd, shell=True)
+            p.communicate()
+            if p.returncode != 0:
+               raise StageCmdFailedError(self)
+ 
             self.save()
 
             self.project.logger.debug(u'\'{}\' was reproduced'.format(self.relpath))

dvc/system.py

@@ -1,14 +1,13 @@
 import ctypes
 import os
 import re
+import subprocess
 from builtins import str
 
 if os.name == 'nt':
     from ctypes import create_unicode_buffer, windll
     import ntfsutils.hardlink as winlink
 
-from dvc.executor import Executor
-
 
 class System(object):
     SYMLINK_OUTPUT = '<SYMLINK>'
@@ -37,8 +36,9 @@ class System(object):
 
     @staticmethod
     def _get_symlink_string(path):
-        code, output, _ = Executor.exec_cmd(["dir", path], shell=True)
-        if code != 0:
+        p = subprocess.Popen(["dir", path], shell=True)
+        output, _ = p.communicate()
+        if p.returncode != 0 or output == None:
             return None
 
         lines = output.split('\n')

tests/test_executor.py

@@ -1,49 +0,0 @@
-import os
-import shutil
-import tempfile
-from unittest import TestCase
-
-from dvc.executor import Executor, ExecutorError
-from dvc.system import System
-
-
-class TestExecutor(TestCase):
-    def setUp(self):
-        self.test_dir = System.get_long_path(tempfile.mkdtemp())
-
-        self.code_file = 'myfile.py'
-        fd = open(self.code_file, 'w')
-        fd.write('import sys\nsys.stdout.write("111")\nsys.stderr.write("222")\n')
-        fd.close()
-
-    def tearDown(self):
-        shutil.rmtree(self.test_dir)
-        if os.path.exists('myfile.py'):
-            os.remove('myfile.py')
-
-    def test_suppress_output(self):
-        output = Executor.exec_cmd_only_success(['python', self.code_file])
-        self.assertEqual(output, '111')
-
-    def test_output_to_std_console(self):
-        # This output "111" in stdout and "222" are not suppressible.
-        # You will see the outputs when the unit-test runs.
-        output = Executor.exec_cmd_only_success(['python', self.code_file], '-', '-')
-        self.assertEqual(output, '')
-
-    def test_custom_file_outputs(self):
-        stdout_file = os.path.join(self.test_dir, 'stdout.txt')
-        stderr_file = os.path.join(self.test_dir, 'stderr.txt')
-
-        output = Executor.exec_cmd_only_success(['python', self.code_file], stdout_file, stderr_file)
-
-        self.assertEqual(output, '')
-        self.assertEqual(open(stdout_file).read(), "111")
-        self.assertEqual(open(stderr_file).read(), "222")
-
-        os.remove(stdout_file)
-        os.remove(stderr_file)
-
-    def test_wrong_command(self):
-        with self.assertRaises(ExecutorError):
-            Executor.exec_cmd_only_success(['--wrong-command--', self.test_dir])