mirror of
https://github.com/Stone-Red-Code/dvc.git
synced 2026-11-02 07:18:25 +01:00
Git wrapper refactoring
This commit is contained in:
@@ -24,7 +24,7 @@ class CmdRepro(CmdRun):
|
||||
pass
|
||||
|
||||
@property
|
||||
def code(self):
|
||||
def code_dependencies(self):
|
||||
return self._code
|
||||
|
||||
def define_args(self, parser):
|
||||
@@ -157,7 +157,7 @@ class ReproChange(object):
|
||||
were_input_files_changed = True
|
||||
|
||||
was_source_code_changed = self.git.were_files_changed(self._data_item.data.relative,
|
||||
self._state.code_sources)
|
||||
self._state.code_dependencies)
|
||||
|
||||
if not force and not was_source_code_changed and not were_input_files_changed:
|
||||
Logger.debug('Data file "{}" is up to date'.format(
|
||||
|
||||
+4
-4
@@ -34,11 +34,11 @@ class CmdRun(CmdBase):
|
||||
parser.add_argument('--output-file', '-o', action='append',
|
||||
help='Declare output file for reproducible cmd')
|
||||
parser.add_argument('--code', '-c', action='append',
|
||||
help='Code file or code directory which produces the output')
|
||||
help='Code dependencies which produce the output')
|
||||
pass
|
||||
|
||||
@property
|
||||
def code(self):
|
||||
def code_dependencies(self):
|
||||
return self.args.code
|
||||
|
||||
@property
|
||||
@@ -109,13 +109,13 @@ class CmdRun(CmdBase):
|
||||
|
||||
data_item.create_symlink()
|
||||
|
||||
nlx_code_sources = map(lambda x: self.git.abs_paths_to_dvc([x])[0], self.code)
|
||||
dvc_code_dependencies = map(lambda x: self.git.abs_paths_to_dvc([x])[0], self.code_dependencies)
|
||||
|
||||
Logger.debug('Create state file "{}"'.format(data_item.state.relative))
|
||||
state_file = StateFile(data_item.state.relative, self.git,
|
||||
input_files,
|
||||
output_files,
|
||||
nlx_code_sources,
|
||||
dvc_code_dependencies,
|
||||
argv=argv)
|
||||
state_file.save()
|
||||
pass
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
import sys
|
||||
import subprocess
|
||||
|
||||
from dvc.exceptions import DvcException
|
||||
|
||||
|
||||
class ExecutorError(DvcException):
|
||||
def __init__(self, msg):
|
||||
DvcException.__init__(self, msg)
|
||||
|
||||
|
||||
class Executor:
|
||||
@staticmethod
|
||||
def exec_cmd(cmd, stdout_file=None, stderr_file=None, cwd=None):
|
||||
stdout, stdout_fd = Executor.output_file(stdout_file)
|
||||
stderr, stderr_fd = Executor.output_file(stderr_file)
|
||||
|
||||
try:
|
||||
p = subprocess.Popen(cmd,
|
||||
cwd=cwd,
|
||||
stdout=stdout,
|
||||
stderr=stderr)
|
||||
out, err = map(lambda s: s.decode().strip('\n\r') if s else '', p.communicate())
|
||||
|
||||
return p.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()
|
||||
pass
|
||||
|
||||
@staticmethod
|
||||
def output_file(output_file, default_output=None):
|
||||
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):
|
||||
code, out, err = Executor.exec_cmd(cmd, stdout_file=stdout_file, stderr_file=stderr_file, cwd=cwd)
|
||||
if code != 0:
|
||||
raise Executor._build_executor_error(cmd, err)
|
||||
return out
|
||||
|
||||
@staticmethod
|
||||
def _build_executor_error(cmd, out):
|
||||
return ExecutorError('Git command error ({}):\n{}'.format(' '.join(cmd), out))
|
||||
+34
-77
@@ -1,15 +1,8 @@
|
||||
import os
|
||||
import sys
|
||||
import subprocess
|
||||
|
||||
from dvc.exceptions import DvcException
|
||||
from dvc.logger import Logger
|
||||
from dvc.config import Config
|
||||
|
||||
|
||||
class GitCmdError(DvcException):
|
||||
def __init__(self, msg):
|
||||
DvcException.__init__(self, msg)
|
||||
from dvc.executor import Executor, ExecutorError
|
||||
|
||||
|
||||
class GitWrapperI(object):
|
||||
@@ -43,50 +36,7 @@ class GitWrapperI(object):
|
||||
|
||||
class GitWrapper(GitWrapperI):
|
||||
def __init__(self):
|
||||
GitWrapperI.__init__(self)
|
||||
|
||||
@staticmethod
|
||||
def exec_cmd(cmd, stdout_file=None, stderr_file=None, cwd=None):
|
||||
stdout_fd = None
|
||||
if stdout_file is not None:
|
||||
if stdout_file == '-':
|
||||
stdout = sys.stdout
|
||||
else:
|
||||
stdout_fd = open(stdout_file, 'w')
|
||||
stdout = stdout_fd
|
||||
else:
|
||||
stdout = subprocess.PIPE
|
||||
|
||||
stderr_fd = None
|
||||
if stderr_file is not None:
|
||||
if stderr_file == '-':
|
||||
stderr = sys.stderr
|
||||
else:
|
||||
stderr_fd = open(stderr_file, 'w')
|
||||
stderr = stderr_fd
|
||||
else:
|
||||
stderr = subprocess.PIPE
|
||||
|
||||
p = subprocess.Popen(cmd, cwd=cwd,
|
||||
stdout=stdout,
|
||||
stderr=stderr)
|
||||
out, err = map(lambda s: s.decode().strip('\n\r') if s else '', p.communicate())
|
||||
|
||||
if stderr_fd:
|
||||
stderr_fd.close()
|
||||
if stdout_fd:
|
||||
stdout_fd.close()
|
||||
|
||||
return p.returncode, out, err
|
||||
|
||||
@staticmethod
|
||||
def exec_cmd_only_success(cmd, stdout_file=None, stderr_file=None, cwd=None):
|
||||
code, out, err = GitWrapper.exec_cmd(cmd, stdout_file=stdout_file, stderr_file=stderr_file, cwd=cwd)
|
||||
if code != 0:
|
||||
if err:
|
||||
sys.stderr.write(err + '\n')
|
||||
raise GitCmdError('Git command error ({}):\n{}'.format(' '.join(cmd), out))
|
||||
return out
|
||||
super(GitWrapper, self).__init__()
|
||||
|
||||
def is_ready_to_go(self):
|
||||
statuses = self.status_files()
|
||||
@@ -100,7 +50,7 @@ class GitWrapper(GitWrapperI):
|
||||
return True
|
||||
|
||||
@property
|
||||
def curr_dir_nlx(self):
|
||||
def curr_dir_dvc(self):
|
||||
return os.path.relpath(os.path.abspath(os.curdir), self.git_dir_abs)
|
||||
|
||||
@property
|
||||
@@ -109,24 +59,24 @@ class GitWrapper(GitWrapperI):
|
||||
return self._git_dir
|
||||
|
||||
try:
|
||||
code, out, err = GitWrapper.exec_cmd(['git', 'rev-parse', '--show-toplevel'])
|
||||
code, out, err = Executor.exec_cmd(['git', 'rev-parse', '--show-toplevel'])
|
||||
|
||||
if code != 0:
|
||||
raise GitCmdError('Git command error - {}'.format(err))
|
||||
raise ExecutorError('Git command error - {}'.format(err))
|
||||
|
||||
self._git_dir = out
|
||||
return self._git_dir
|
||||
except GitCmdError:
|
||||
except ExecutorError:
|
||||
raise
|
||||
except Exception as e:
|
||||
raise GitCmdError('Unable to run git command: {}'.format(e))
|
||||
raise ExecutorError('Unable to run git command: {}'.format(e))
|
||||
pass
|
||||
|
||||
@staticmethod
|
||||
def status_files():
|
||||
code, out, err = GitWrapper.exec_cmd(['git', 'status', '--porcelain'])
|
||||
code, out, err = Executor.exec_cmd(['git', 'status', '--porcelain'])
|
||||
if code != 0:
|
||||
raise GitCmdError('Git command error - {}'.format(err))
|
||||
raise ExecutorError('Git command error - {}'.format(err))
|
||||
|
||||
return GitWrapper.parse_porcelain_files(out)
|
||||
|
||||
@@ -144,16 +94,17 @@ class GitWrapper(GitWrapperI):
|
||||
@property
|
||||
def curr_commit(self):
|
||||
if self._commit is None:
|
||||
code, out, err = GitWrapper.exec_cmd(['git', 'rev-parse', '--short', 'HEAD'])
|
||||
code, out, err = Executor.exec_cmd(['git', 'rev-parse', '--short', 'HEAD'])
|
||||
if code != 0:
|
||||
raise GitCmdError('Git command error - {}'.format(err))
|
||||
raise ExecutorError('Git command error - {}'.format(err))
|
||||
self._commit = out
|
||||
return self._commit
|
||||
|
||||
def commit_all_changes(self, message):
|
||||
GitWrapper.exec_cmd_only_success(['git', 'add', '--all'])
|
||||
out_status = GitWrapper.exec_cmd_only_success(['git', 'status', '--porcelain'])
|
||||
GitWrapper.exec_cmd_only_success(['git', 'commit', '-m', message])
|
||||
@staticmethod
|
||||
def commit_all_changes(message):
|
||||
Executor.exec_cmd_only_success(['git', 'add', '--all'])
|
||||
out_status = Executor.exec_cmd_only_success(['git', 'status', '--porcelain'])
|
||||
Executor.exec_cmd_only_success(['git', 'commit', '-m', message])
|
||||
return GitWrapper.parse_porcelain_files(out_status)
|
||||
|
||||
def commit_all_changes_and_log_status(self, message):
|
||||
@@ -180,7 +131,7 @@ class GitWrapper(GitWrapperI):
|
||||
|
||||
return result
|
||||
|
||||
def nlx_paths_to_abs(self, files):
|
||||
def dvc_paths_to_abs(self, files):
|
||||
results = []
|
||||
|
||||
for file in files:
|
||||
@@ -188,21 +139,14 @@ class GitWrapper(GitWrapperI):
|
||||
|
||||
return results
|
||||
|
||||
def were_files_changed(self, file, code_sources):
|
||||
commit = self.exec_cmd_only_success(['git', 'log', '-1', '--pretty=format:"%h"', file])
|
||||
def were_files_changed(self, file, code_dependencies):
|
||||
commit = Executor.exec_cmd_only_success(['git', 'log', '-1', '--pretty=format:"%h"', file])
|
||||
commit = commit.strip('"')
|
||||
|
||||
changed_files = self.exec_cmd_only_success(['git', 'diff', '--name-only', 'HEAD', commit])
|
||||
changed_files = Executor.exec_cmd_only_success(['git', 'diff', '--name-only', 'HEAD', commit])
|
||||
changed_files = changed_files.strip('"')
|
||||
|
||||
code_sources_abs = self.nlx_paths_to_abs(code_sources)
|
||||
code_files = []
|
||||
code_dirs = []
|
||||
for code in code_sources_abs:
|
||||
if os.path.isdir(code):
|
||||
code_dirs.append(code)
|
||||
else:
|
||||
code_files.append(code)
|
||||
code_files, code_dirs = self.separate_dependency_files_and_dirs(code_dependencies)
|
||||
|
||||
code_files_set = set(code_files)
|
||||
for changed_file in changed_files.split('\n'):
|
||||
@@ -215,3 +159,16 @@ class GitWrapper(GitWrapperI):
|
||||
return True
|
||||
|
||||
return False
|
||||
|
||||
def separate_dependency_files_and_dirs(self, code_dependencies):
|
||||
code_files = []
|
||||
code_dirs = []
|
||||
|
||||
code_dependencies_abs = self.dvc_paths_to_abs(code_dependencies)
|
||||
for code in code_dependencies_abs:
|
||||
if os.path.isdir(code):
|
||||
code_dirs.append(code)
|
||||
else:
|
||||
code_files.append(code)
|
||||
|
||||
return code_files, code_dirs
|
||||
|
||||
+9
-9
@@ -23,11 +23,11 @@ class StateFile(object):
|
||||
PARAM_CREATED_AT = 'CreatedAt'
|
||||
PARAM_INPUT_FILES = 'InputFiles'
|
||||
PARAM_OUTPUT_FILES = 'OutputFiles'
|
||||
PARAM_CODE_SOURCES = 'CodeSources'
|
||||
PARAM_CODE_DEPENDENCIES = 'CodeDependencies'
|
||||
PARAM_NOT_REPRODUCIBLE = 'NotReproducible'
|
||||
|
||||
def __init__(self, file, git, input_files, output_files,
|
||||
code_sources=[],
|
||||
code_dependencies=[],
|
||||
is_reproducible=True,
|
||||
argv=sys.argv,
|
||||
norm_argv=None,
|
||||
@@ -38,7 +38,7 @@ class StateFile(object):
|
||||
self.input_files = input_files
|
||||
self.output_files = output_files
|
||||
self.is_reproducible = is_reproducible
|
||||
self.code_sources = code_sources
|
||||
self.code_dependencies = code_dependencies
|
||||
|
||||
self.argv = argv
|
||||
if norm_argv:
|
||||
@@ -51,7 +51,7 @@ class StateFile(object):
|
||||
if cwd:
|
||||
self.cwd = cwd
|
||||
else:
|
||||
self.cwd = self.get_nlx_path()
|
||||
self.cwd = self.get_dvc_path()
|
||||
pass
|
||||
|
||||
@staticmethod
|
||||
@@ -63,7 +63,7 @@ class StateFile(object):
|
||||
git,
|
||||
data.get(StateFile.PARAM_INPUT_FILES, []),
|
||||
data.get(StateFile.PARAM_OUTPUT_FILES, []),
|
||||
data.get(StateFile.PARAM_CODE_SOURCES, []),
|
||||
data.get(StateFile.PARAM_CODE_DEPENDENCIES, []),
|
||||
not data.get(StateFile.PARAM_NOT_REPRODUCIBLE, False),
|
||||
data.get(StateFile.PARAM_ARGV),
|
||||
data.get(StateFile.PARAM_NORM_ARGV),
|
||||
@@ -80,7 +80,7 @@ class StateFile(object):
|
||||
self.PARAM_CREATED_AT: self.created_at,
|
||||
self.PARAM_INPUT_FILES: self.input_files,
|
||||
self.PARAM_OUTPUT_FILES: self.output_files,
|
||||
self.PARAM_CODE_SOURCES: self.code_sources
|
||||
self.PARAM_CODE_DEPENDENCIES: self.code_dependencies
|
||||
}
|
||||
|
||||
if not self.is_reproducible:
|
||||
@@ -107,14 +107,14 @@ class StateFile(object):
|
||||
for arg in self.argv [1:]:
|
||||
if os.path.isfile(arg):
|
||||
path = os.path.abspath(arg)
|
||||
nlx_path = os.path.relpath(path, self.git.git_dir_abs)
|
||||
result.append(nlx_path)
|
||||
dvc_path = os.path.relpath(path, self.git.git_dir_abs)
|
||||
result.append(dvc_path)
|
||||
else:
|
||||
result.append(arg)
|
||||
|
||||
return result
|
||||
|
||||
def get_nlx_path(self):
|
||||
def get_dvc_path(self):
|
||||
pwd = os.path.realpath(os.curdir)
|
||||
if not pwd.startswith(self.git.git_dir_abs):
|
||||
raise StateFileError('the file cannot be created outside of a git repository')
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
import os
|
||||
import shutil
|
||||
import tempfile
|
||||
from unittest import TestCase
|
||||
|
||||
from dvc.executor import Executor, ExecutorError
|
||||
|
||||
|
||||
class TestExecutor(TestCase):
|
||||
def setUp(self):
|
||||
self.test_dir = tempfile.mkdtemp()
|
||||
|
||||
self.file1 = 'file1.txt'
|
||||
fd1 = open(os.path.join(self.test_dir, self.file1), 'w+')
|
||||
fd1.write('this is file 1')
|
||||
fd1.close()
|
||||
|
||||
self.file2 = 'f.csv'
|
||||
fd2 = open(os.path.join(self.test_dir, self.file2), 'w+')
|
||||
fd2.write('this is file 222')
|
||||
fd2.close()
|
||||
|
||||
def tearDown(self):
|
||||
shutil.rmtree(self.test_dir)
|
||||
|
||||
def test_suppress_output(self):
|
||||
output = Executor.exec_cmd_only_success(['ls', self.test_dir])
|
||||
output_set = set(output.split())
|
||||
self.assertEqual(output_set, {self.file1, self.file2})
|
||||
|
||||
def test_output_to_std_console(self):
|
||||
output = Executor.exec_cmd_only_success(['ls', self.test_dir], '-', '-')
|
||||
self.assertEqual(output, '')
|
||||
|
||||
def test_custom_files_output(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(['ls', self.test_dir], stdout_file, stderr_file)
|
||||
|
||||
self.assertEqual(output, '')
|
||||
|
||||
output_set = set(open(stdout_file).read().split())
|
||||
expected = {self.file1, self.file2, os.path.basename(stdout_file), os.path.basename(stderr_file)}
|
||||
self.assertEqual(output_set, expected)
|
||||
|
||||
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])
|
||||
Reference in New Issue
Block a user