From f624110686bee433eb3424d57fd42995436af4d1 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sat, 18 Nov 2017 17:23:53 -0800 Subject: [PATCH 1/8] Error message if not args specified --- dvc/command/remove.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dvc/command/remove.py b/dvc/command/remove.py index 24c365467..49460d322 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -8,6 +8,14 @@ class CmdRemove(Traverse): def __init__(self, settings): super(CmdRemove, self).__init__(settings, "remove") + def run(self): + if not self.parsed_args.target: + Logger.error('Nothing specified, nothing removed.') + return 0 + + super(CmdRemove, self).run() + pass + def process_file(self, target): Logger.debug(u'[Cmd-Remove] Remove file {}.'.format(target)) From 847a5e2716f02b7f22f06659db1b0fdfe1c97f7f Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sun, 19 Nov 2017 02:41:59 -0800 Subject: [PATCH 2/8] Add branch and new-branch options --- dvc/cli.py | 19 ++++++++++++++++--- dvc/command/base.py | 33 +++++++++++++++++++++++++++++---- dvc/command/remove.py | 5 ++--- dvc/git_wrapper.py | 12 +++++++++--- dvc/main.py | 2 +- 5 files changed, 57 insertions(+), 14 deletions(-) diff --git a/dvc/cli.py b/dvc/cli.py index df02b5ea5..9db4f3841 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -25,21 +25,34 @@ from dvc import VERSION def parse_args(argv=None): # Common args parent_parser = argparse.ArgumentParser(add_help=False) - parent_parser.add_argument('-q', + parent_parser.add_argument( + '-q', '--quiet', action='store_true', default=False, help='Be quiet.') - parent_parser.add_argument('-v', + parent_parser.add_argument( + '-v', '--verbose', action='store_true', default=False, help='Be verbose.') - parent_parser.add_argument('-G', + parent_parser.add_argument( + '-G', '--no-git-actions', action='store_true', default=False, help='Skip all git actions including reproducibility check and commits.') + parent_parser.add_argument( + '-b', + '--branch', + metavar='BRANCH', + help='Execute a command in the branch.') + parent_parser.add_argument( + '-n', + '--new-branch', + metavar='BRANCH', + help='Create a new branch and execute a command in the branch.') # Main parser desc = 'Data Version Control' diff --git a/dvc/command/base.py b/dvc/command/base.py index b28d13206..b545b675f 100644 --- a/dvc/command/base.py +++ b/dvc/command/base.py @@ -1,9 +1,5 @@ -import argparse import fasteners -from multiprocessing import cpu_count - -from dvc.config import ConfigError from dvc.exceptions import DvcException from dvc.logger import Logger @@ -36,6 +32,30 @@ class DvcLock(object): self.lock.release() +class ForeignBranch(object): + def __init__(self, branch, new_branch, git): + self.branch = branch + self.new_branch = new_branch + self.git = git + + if branch and new_branch: + raise DvcLockerError("Commands conflict: --branch and --new-branch cannot be used at the same command") + + self.perform_action = branch or new_branch + + def __enter__(self): + if self.perform_action: + if self.branch: + self.git.checkout(self.branch) + else: + self.git.checkout_new(self.new_branch) + return self + + def __exit__(self, type, value, traceback): + if self.perform_action: + self.git.checkout() + + class CmdBase(object): def __init__(self, settings): self._settings = settings @@ -99,5 +119,10 @@ class CmdBase(object): def not_committed_changes_warning(): Logger.warn('changes were not committed to git') + def run_cmd(self): + with ForeignBranch(self.parsed_args.branch, self.parsed_args.new_branch, self.git): + return self.run() + + # Abstract method that has to be implemented by any inheritance class def run(self): pass diff --git a/dvc/command/remove.py b/dvc/command/remove.py index 49460d322..00ec8b058 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -11,10 +11,9 @@ class CmdRemove(Traverse): def run(self): if not self.parsed_args.target: Logger.error('Nothing specified, nothing removed.') - return 0 + return 1 - super(CmdRemove, self).run() - pass + return super(CmdRemove, self).run() def process_file(self, target): Logger.debug(u'[Cmd-Remove] Remove file {}.'.format(target)) diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index aca0162b6..f07fffba1 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -317,9 +317,15 @@ class GitWrapper(GitWrapperI): Executor.exec_cmd_only_success(cmd.split()) @staticmethod - def checkout(commit): - Logger.debug('[dvc-git] Checkout {}'.format(commit)) - cmd = 'git checkout {}'.format(commit) + def checkout(commit_or_branch): + Logger.debug('[dvc-git] Checkout {}'.format(commit_or_branch)) + cmd = 'git checkout {}'.format(commit_or_branch) + Executor.exec_cmd(cmd.split()) + + @staticmethod + def checkout_new(branch): + Logger.debug('[dvc-git] Checkout new branch {}'.format(branch)) + cmd = 'git checkout -b{}'.format(branch) Executor.exec_cmd(cmd.split()) @staticmethod diff --git a/dvc/main.py b/dvc/main.py index 22f9af212..276d60cd8 100644 --- a/dvc/main.py +++ b/dvc/main.py @@ -18,7 +18,7 @@ def main(): return 255 try: - ret = instance.run() + ret = instance.run_cmd() except Exception as e: exc_info = settings.parsed_args.verbose Logger.error("{} error: {}".format(instance.__class__.__name__, e), exc_info=exc_info) From 1349589ec76eec393867c4ed32b8cf6130ab11af Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sun, 19 Nov 2017 03:12:37 -0800 Subject: [PATCH 3/8] Move CmdBase class to common dir --- dvc/command/checkout.py | 2 +- dvc/command/common/__init__.py | 0 dvc/command/{ => common}/base.py | 0 dvc/command/config.py | 2 +- dvc/command/data_sync.py | 2 +- dvc/command/find.py | 2 +- dvc/command/gc.py | 2 +- dvc/command/import_file.py | 2 +- dvc/command/init.py | 2 +- dvc/command/instance_create.py | 2 +- dvc/command/lock.py | 2 +- dvc/command/merge.py | 4 ++-- dvc/command/repro.py | 2 +- dvc/command/run.py | 4 +--- dvc/command/show_pipeline.py | 1 - dvc/command/show_workflow.py | 2 +- dvc/command/traverse.py | 2 +- 17 files changed, 15 insertions(+), 18 deletions(-) create mode 100644 dvc/command/common/__init__.py rename dvc/command/{ => common}/base.py (100%) diff --git a/dvc/command/checkout.py b/dvc/command/checkout.py index eb51a824b..d82230f17 100644 --- a/dvc/command/checkout.py +++ b/dvc/command/checkout.py @@ -1,6 +1,6 @@ import os -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.logger import Logger from dvc.system import System diff --git a/dvc/command/common/__init__.py b/dvc/command/common/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/dvc/command/base.py b/dvc/command/common/base.py similarity index 100% rename from dvc/command/base.py rename to dvc/command/common/base.py diff --git a/dvc/command/config.py b/dvc/command/config.py index 8db88b509..194e2215c 100644 --- a/dvc/command/config.py +++ b/dvc/command/config.py @@ -1,7 +1,7 @@ import os import configobj -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.logger import Logger from dvc.config import Config diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index 71f060877..a38c4e747 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -1,4 +1,4 @@ -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.logger import Logger import dvc.data_cloud as cloud diff --git a/dvc/command/find.py b/dvc/command/find.py index bf837d94d..d99ee159e 100644 --- a/dvc/command/find.py +++ b/dvc/command/find.py @@ -1,5 +1,5 @@ from dvc import utils -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.executor import ExecutorError from dvc.logger import Logger diff --git a/dvc/command/gc.py b/dvc/command/gc.py index ed39afc6a..acc24587d 100644 --- a/dvc/command/gc.py +++ b/dvc/command/gc.py @@ -1,6 +1,6 @@ import os -from dvc.command.base import DvcLock +from dvc.command.common.base import DvcLock from dvc.command.traverse import Traverse from dvc.logger import Logger from dvc.path.data_item import DataItemError diff --git a/dvc/command/import_file.py b/dvc/command/import_file.py index ff9e53126..7f50d935c 100644 --- a/dvc/command/import_file.py +++ b/dvc/command/import_file.py @@ -8,7 +8,7 @@ except ImportError: # pylint: disable=no-name-in-module, import-error from urllib.parse import urlparse -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.data_cloud import sizeof_fmt, file_md5 from dvc.logger import Logger from dvc.exceptions import DvcException diff --git a/dvc/command/init.py b/dvc/command/init.py index b5e3b5e40..44b807ffa 100644 --- a/dvc/command/init.py +++ b/dvc/command/init.py @@ -1,7 +1,7 @@ import os from pathlib import Path -from dvc.command.base import CmdBase +from dvc.command.common.base import CmdBase from dvc.logger import Logger from dvc.config import Config from dvc.exceptions import DvcException diff --git a/dvc/command/instance_create.py b/dvc/command/instance_create.py index 35237b152..ecd0a9412 100644 --- a/dvc/command/instance_create.py +++ b/dvc/command/instance_create.py @@ -1,5 +1,5 @@ from dvc.cloud.instance_manager import InstanceManager -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.exceptions import DvcException from dvc.logger import Logger diff --git a/dvc/command/lock.py b/dvc/command/lock.py index 9b8f8a674..4ac0b12e7 100644 --- a/dvc/command/lock.py +++ b/dvc/command/lock.py @@ -1,4 +1,4 @@ -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.logger import Logger from dvc.state_file import StateFile diff --git a/dvc/command/merge.py b/dvc/command/merge.py index ed30906e9..edaf39310 100644 --- a/dvc/command/merge.py +++ b/dvc/command/merge.py @@ -1,13 +1,13 @@ import os -import json -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.config import ConfigI from dvc.logger import Logger from dvc.state_file import StateFile from dvc.path.data_item import DataItem from dvc.system import System + class CmdMerge(CmdBase): def __init__(self, settings): super(CmdMerge, self).__init__(settings) diff --git a/dvc/command/repro.py b/dvc/command/repro.py index ff64210b3..433b5d6ab 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -1,7 +1,7 @@ import os import copy -from dvc.command.base import DvcLock +from dvc.command.common.base import DvcLock from dvc.command.import_file import CmdImportFile from dvc.command.run import CmdRun from dvc.logger import Logger diff --git a/dvc/command/run.py b/dvc/command/run.py index 30c081277..bb4595ad1 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -1,11 +1,9 @@ import os -import sys -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.exceptions import DvcException from dvc.git_wrapper import GitWrapper from dvc.logger import Logger -from dvc.path.data_item import DataDirError, NotInGitDirError from dvc.repository_change import RepositoryChange from dvc.state_file import StateFile from dvc.utils import cached_property diff --git a/dvc/command/show_pipeline.py b/dvc/command/show_pipeline.py index e54b31f55..1fe8c916c 100644 --- a/dvc/command/show_pipeline.py +++ b/dvc/command/show_pipeline.py @@ -2,7 +2,6 @@ import os import networkx as nx from dvc.command.traverse import Traverse -from dvc.command.init import CmdInit from dvc.logger import Logger from dvc.state_file import StateFile diff --git a/dvc/command/show_workflow.py b/dvc/command/show_workflow.py index 80322d99c..4d9d794b1 100644 --- a/dvc/command/show_workflow.py +++ b/dvc/command/show_workflow.py @@ -1,6 +1,6 @@ import os -from dvc.command.base import CmdBase +from dvc.command.common.base import CmdBase from dvc.logger import Logger diff --git a/dvc/command/traverse.py b/dvc/command/traverse.py index b4be15839..2d562a144 100644 --- a/dvc/command/traverse.py +++ b/dvc/command/traverse.py @@ -1,6 +1,6 @@ import os -from dvc.command.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase, DvcLock from dvc.exceptions import DvcException from dvc.logger import Logger from dvc.path.data_item import DataItemError From 47e72559f2bfb76161def02d2e43d7559f252626 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sun, 19 Nov 2017 03:47:41 -0800 Subject: [PATCH 4/8] Extract small classes to files --- dvc/command/common/base.py | 59 ++-------------------------- dvc/command/common/branch_changer.py | 28 +++++++++++++ dvc/command/common/common_error.py | 6 +++ dvc/command/common/dvc_lock.py | 26 ++++++++++++ dvc/git_wrapper.py | 13 ++---- 5 files changed, 67 insertions(+), 65 deletions(-) create mode 100644 dvc/command/common/branch_changer.py create mode 100644 dvc/command/common/common_error.py create mode 100644 dvc/command/common/dvc_lock.py diff --git a/dvc/command/common/base.py b/dvc/command/common/base.py index b545b675f..df96aac6e 100644 --- a/dvc/command/common/base.py +++ b/dvc/command/common/base.py @@ -1,59 +1,6 @@ -import fasteners - -from dvc.exceptions import DvcException from dvc.logger import Logger - - -class CmdBaseError(DvcException): - def __init__(self, msg): - super(CmdBaseError, self).__init__('{}'.format(msg)) - - -class DvcLockerError(CmdBaseError): - def __init__(self, msg): - super(DvcLockerError, self).__init__('DVC locker error: {}'.format(msg)) - - -class DvcLock(object): - def __init__(self, is_locker, git): - self.is_locker = is_locker - self.git = git - self.lock = None - - def __enter__(self): - if self.is_locker: - self.lock = fasteners.InterProcessLock(self.git.lock_file) - if not self.lock.acquire(timeout=5): - raise DvcLockerError('Cannot perform the cmd since DVC is busy and locked. Please retry the cmd later.') - return self.lock - - def __exit__(self, type, value, traceback): - if self.is_locker: - self.lock.release() - - -class ForeignBranch(object): - def __init__(self, branch, new_branch, git): - self.branch = branch - self.new_branch = new_branch - self.git = git - - if branch and new_branch: - raise DvcLockerError("Commands conflict: --branch and --new-branch cannot be used at the same command") - - self.perform_action = branch or new_branch - - def __enter__(self): - if self.perform_action: - if self.branch: - self.git.checkout(self.branch) - else: - self.git.checkout_new(self.new_branch) - return self - - def __exit__(self, type, value, traceback): - if self.perform_action: - self.git.checkout() +from dvc.command.common.branch_changer import BranchChanger +from dvc.command.common.dvc_lock import DvcLock class CmdBase(object): @@ -120,7 +67,7 @@ class CmdBase(object): Logger.warn('changes were not committed to git') def run_cmd(self): - with ForeignBranch(self.parsed_args.branch, self.parsed_args.new_branch, self.git): + with BranchChanger(self.parsed_args.branch, self.parsed_args.new_branch, self.git): return self.run() # Abstract method that has to be implemented by any inheritance class diff --git a/dvc/command/common/branch_changer.py b/dvc/command/common/branch_changer.py new file mode 100644 index 000000000..9a40e9a18 --- /dev/null +++ b/dvc/command/common/branch_changer.py @@ -0,0 +1,28 @@ +from dvc.command.common.common_error import CmdCommonError + + +class BranchChangerError(CmdCommonError): + def __init__(self, msg): + super(CmdCommonError, self).__init__('DVC locker error: {}'.format(msg)) + + +class BranchChanger(object): + def __init__(self, branch, new_branch, git): + if branch and new_branch: + raise BranchChangerError("Commands conflict: --branch and --new-branch cannot be used at the same command") + + self.perform_action = branch or new_branch + + self.branch = branch if branch else new_branch + self.create_new = new_branch is not None + self.git = git + + def __enter__(self): + if self.perform_action: + print('========PERFORM') + self.git.checkout(self.branch, self.create_new) + return self + + def __exit__(self, type, value, traceback): + if self.perform_action: + self.git.checkout_previous() diff --git a/dvc/command/common/common_error.py b/dvc/command/common/common_error.py new file mode 100644 index 000000000..3013a73bb --- /dev/null +++ b/dvc/command/common/common_error.py @@ -0,0 +1,6 @@ +from dvc.exceptions import DvcException + + +class CmdCommonError(DvcException): + def __init__(self, msg): + super(CmdCommonError, self).__init__('{}'.format(msg)) diff --git a/dvc/command/common/dvc_lock.py b/dvc/command/common/dvc_lock.py new file mode 100644 index 000000000..e65d638c8 --- /dev/null +++ b/dvc/command/common/dvc_lock.py @@ -0,0 +1,26 @@ +import fasteners + +from dvc.command.common.common_error import CmdCommonError + + +class DvcLockerError(CmdCommonError): + def __init__(self, msg): + super(DvcLockerError, self).__init__('DVC locker error: {}'.format(msg)) + + +class DvcLock(object): + def __init__(self, is_locker, git): + self.is_locker = is_locker + self.git = git + self.lock = None + + def __enter__(self): + if self.is_locker: + self.lock = fasteners.InterProcessLock(self.git.lock_file) + if not self.lock.acquire(timeout=5): + raise DvcLockerError('Cannot perform the cmd since DVC is busy and locked. Please retry the cmd later.') + return self.lock + + def __exit__(self, type, value, traceback): + if self.is_locker: + self.lock.release() diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index f07fffba1..02b5af2de 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -317,15 +317,10 @@ class GitWrapper(GitWrapperI): Executor.exec_cmd_only_success(cmd.split()) @staticmethod - def checkout(commit_or_branch): - Logger.debug('[dvc-git] Checkout {}'.format(commit_or_branch)) - cmd = 'git checkout {}'.format(commit_or_branch) - Executor.exec_cmd(cmd.split()) - - @staticmethod - def checkout_new(branch): - Logger.debug('[dvc-git] Checkout new branch {}'.format(branch)) - cmd = 'git checkout -b{}'.format(branch) + def checkout(commit_or_branch, create_new=False): + prefix = '-b ' if create_new else '' + Logger.debug('[dvc-git] Checkout {}{}'.format(prefix, commit_or_branch)) + cmd = 'git checkout {}{}'.format(prefix, commit_or_branch) Executor.exec_cmd(cmd.split()) @staticmethod From db625e8213c210edc4609804fbc99158e3eb149f Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sun, 19 Nov 2017 04:14:12 -0800 Subject: [PATCH 5/8] Use DvcLock from CmdBase class only --- dvc/command/checkout.py | 23 +++++++------ dvc/command/common/base.py | 19 +++++------ dvc/command/common/branch_changer.py | 1 - dvc/command/common/dvc_lock.py | 4 ++- dvc/command/config.py | 8 ++--- dvc/command/data_sync.py | 18 ++++------- dvc/command/find.py | 29 ++++++++--------- dvc/command/gc.py | 24 +++++++------- dvc/command/import_file.py | 48 +++++++++++++--------------- dvc/command/init.py | 4 +++ dvc/command/instance_create.py | 29 ++++++++--------- dvc/command/lock.py | 5 ++- dvc/command/merge.py | 15 ++++----- dvc/command/repro.py | 29 ++++++++--------- dvc/command/run.py | 20 ++++++------ dvc/command/traverse.py | 8 ++--- 16 files changed, 133 insertions(+), 151 deletions(-) diff --git a/dvc/command/checkout.py b/dvc/command/checkout.py index d82230f17..efa58fa9c 100644 --- a/dvc/command/checkout.py +++ b/dvc/command/checkout.py @@ -1,6 +1,6 @@ import os -from dvc.command.common.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase from dvc.logger import Logger from dvc.system import System @@ -34,19 +34,18 @@ class CmdCheckout(CmdBase): Logger.info('Checkout \'{}\''.format(item.data.relative)) def run(self): - with DvcLock(self.is_locker, self.git): - try: - states = self.git.state_files_for_previous_commit() - prev_items = self.settings.path_factory.data_items_from_states(states, existing=False) - except Exception as ex: - Logger.error(u'Unable to get data files from previous commit: {}'.format(ex)) - return 1 + try: + states = self.git.state_files_for_previous_commit() + prev_items = self.settings.path_factory.data_items_from_states(states, existing=False) + except Exception as ex: + Logger.error(u'Unable to get data files from previous commit: {}'.format(ex)) + return 1 - curr_items = self.settings.path_factory.all_existing_data_items() - self.checkout(curr_items) + curr_items = self.settings.path_factory.all_existing_data_items() + self.checkout(curr_items) - self.remove_files(list(set(prev_items) - set(curr_items))) - return 0 + self.remove_files(list(set(prev_items) - set(curr_items))) + return 0 @staticmethod def remove_files(removed_items_set): diff --git a/dvc/command/common/base.py b/dvc/command/common/base.py index df96aac6e..3b1bbd5fe 100644 --- a/dvc/command/common/base.py +++ b/dvc/command/common/base.py @@ -45,12 +45,6 @@ class CmdBase(object): def set_git_action(self, value): self.parsed_args.no_git_actions = not value - @property - def is_locker(self): - if 'no_lock' in self.parsed_args.__dict__: - return not self.parsed_args.no_lock - return True - def set_locker(self, value): self.parsed_args.no_lock = value @@ -67,9 +61,16 @@ class CmdBase(object): Logger.warn('changes were not committed to git') def run_cmd(self): - with BranchChanger(self.parsed_args.branch, self.parsed_args.new_branch, self.git): - return self.run() + with DvcLock(self.is_locker, self.git): + with BranchChanger(self.parsed_args.branch, self.parsed_args.new_branch, self.git): + return self.run() - # Abstract method that has to be implemented by any inheritance class + # Abstract methods that have to be implemented by any inheritance class def run(self): pass + + @property + def is_locker(self): + if 'no_lock' in self.parsed_args.__dict__: + return not self.parsed_args.no_lock + return True diff --git a/dvc/command/common/branch_changer.py b/dvc/command/common/branch_changer.py index 9a40e9a18..819b7a33b 100644 --- a/dvc/command/common/branch_changer.py +++ b/dvc/command/common/branch_changer.py @@ -19,7 +19,6 @@ class BranchChanger(object): def __enter__(self): if self.perform_action: - print('========PERFORM') self.git.checkout(self.branch, self.create_new) return self diff --git a/dvc/command/common/dvc_lock.py b/dvc/command/common/dvc_lock.py index e65d638c8..973d4afce 100644 --- a/dvc/command/common/dvc_lock.py +++ b/dvc/command/common/dvc_lock.py @@ -9,6 +9,8 @@ class DvcLockerError(CmdCommonError): class DvcLock(object): + TIMEOUT = 5 + def __init__(self, is_locker, git): self.is_locker = is_locker self.git = git @@ -17,7 +19,7 @@ class DvcLock(object): def __enter__(self): if self.is_locker: self.lock = fasteners.InterProcessLock(self.git.lock_file) - if not self.lock.acquire(timeout=5): + if not self.lock.acquire(timeout=self.TIMEOUT): raise DvcLockerError('Cannot perform the cmd since DVC is busy and locked. Please retry the cmd later.') return self.lock diff --git a/dvc/command/config.py b/dvc/command/config.py index 194e2215c..b948529d1 100644 --- a/dvc/command/config.py +++ b/dvc/command/config.py @@ -1,7 +1,7 @@ import os import configobj -from dvc.command.common.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase from dvc.logger import Logger from dvc.config import Config @@ -75,7 +75,7 @@ class CmdConfig(CmdBase): return 0 - def _run(self): + def run(self): if self.check_opt() != 0: return 1 @@ -86,7 +86,3 @@ class CmdConfig(CmdBase): return self.show() return self.set() - - def run(self): - with DvcLock(self.is_locker, self.git): - return self._run() diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index a38c4e747..f8b188fc9 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -1,4 +1,4 @@ -from dvc.command.common.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase from dvc.logger import Logger import dvc.data_cloud as cloud @@ -9,8 +9,7 @@ class CmdDataPull(CmdBase): super(CmdDataPull, self).__init__(settings) def run(self): - with DvcLock(self.is_locker, self.git): - self.cloud.pull(self.parsed_args.targets, self.parsed_args.jobs) + self.cloud.pull(self.parsed_args.targets, self.parsed_args.jobs) class CmdDataPush(CmdBase): @@ -18,8 +17,8 @@ class CmdDataPush(CmdBase): super(CmdDataPush, self).__init__(settings) def run(self): - with DvcLock(self.is_locker, self.git): - self.cloud.push(self.parsed_args.targets, self.parsed_args.jobs) + self.cloud.push(self.parsed_args.targets, self.parsed_args.jobs) + class CmdDataStatus(CmdBase): def __init__(self, settings): @@ -41,9 +40,6 @@ class CmdDataStatus(CmdBase): Logger.info('\t{}\t{}'.format(prefix_map[ret], target.data.dvc)) def run(self): - with DvcLock(self.is_locker, self.git): - status = self.cloud.status(self.parsed_args.targets, self.parsed_args.jobs) - - self._show(status) - - return 0 + status = self.cloud.status(self.parsed_args.targets, self.parsed_args.jobs) + self._show(status) + return 0 diff --git a/dvc/command/find.py b/dvc/command/find.py index d99ee159e..3bb708c02 100644 --- a/dvc/command/find.py +++ b/dvc/command/find.py @@ -1,5 +1,5 @@ from dvc import utils -from dvc.command.common.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase from dvc.executor import ExecutorError from dvc.logger import Logger @@ -9,23 +9,22 @@ class CmdFind(CmdBase): super(CmdFind, self).__init__(settings) def run(self): - with DvcLock(self.is_locker, self.git): - fname = self.parsed_args.target - branch_name = self.parsed_args.branch_name + fname = self.parsed_args.target + branch_name = self.parsed_args.branch_name - branches = self.git.branches(branch_name) - metrics = [self.read_metrics(fname, b) for b in branches] + branches = self.git.branches(branch_name) + metrics = [self.read_metrics(fname, b) for b in branches] - zipped = zip(metrics, branches) - res = self.filter(zipped, self.parsed_args.criteria) - res = filter(lambda x: x[0] is not None, res) + zipped = zip(metrics, branches) + res = self.filter(zipped, self.parsed_args.criteria) + res = filter(lambda x: x[0] is not None, res) - for (val, branch) in res: - if self.parsed_args.show_value: - print('{}: {}'.format(branch, val)) - else: - print(branch) - return 0 + for (val, branch) in res: + if self.parsed_args.show_value: + print('{}: {}'.format(branch, val)) + else: + print(branch) + return 0 @staticmethod def filter(zipped, criteria): diff --git a/dvc/command/gc.py b/dvc/command/gc.py index acc24587d..7f73e925d 100644 --- a/dvc/command/gc.py +++ b/dvc/command/gc.py @@ -1,6 +1,5 @@ import os -from dvc.command.common.base import DvcLock from dvc.command.traverse import Traverse from dvc.logger import Logger from dvc.path.data_item import DataItemError @@ -13,20 +12,19 @@ class CmdGC(Traverse): self.clist = [] def run(self): - with DvcLock(self.is_locker, self.git): - if not self._traverse(self.git.git_dir_abs): - Logger.error('Failed to collect used cache') - return 1 + if not self._traverse(self.git.git_dir_abs): + Logger.error('Failed to collect used cache') + return 1 - for cache in os.listdir(ConfigI.CACHE_DIR): - fname = os.path.join(ConfigI.CACHE_DIR, cache) - if fname in self.clist: - continue - os.remove(fname) - self._remove_cloud_cache(self.settings.path_factory.data_item('.empty', fname)) - Logger.info('Cache \'{}\' was removed'.format(fname)) + for cache in os.listdir(ConfigI.CACHE_DIR): + fname = os.path.join(ConfigI.CACHE_DIR, cache) + if fname in self.clist: + continue + os.remove(fname) + self._remove_cloud_cache(self.settings.path_factory.data_item('.empty', fname)) + Logger.info('Cache \'{}\' was removed'.format(fname)) - return 0 + return 0 def process_file(self, target): Logger.debug(u'[Cmd-GC] GC file {}.'.format(target)) diff --git a/dvc/command/import_file.py b/dvc/command/import_file.py index 7f50d935c..f26e2cfc7 100644 --- a/dvc/command/import_file.py +++ b/dvc/command/import_file.py @@ -1,5 +1,4 @@ import os -import re try: from urlparse import urlparse @@ -8,12 +7,11 @@ except ImportError: # pylint: disable=no-name-in-module, import-error from urllib.parse import urlparse -from dvc.command.common.base import CmdBase, DvcLock -from dvc.data_cloud import sizeof_fmt, file_md5 +from dvc.command.common.base import CmdBase from dvc.logger import Logger from dvc.exceptions import DvcException from dvc.state_file import StateFile -from dvc.system import System + class ImportFileError(DvcException): def __init__(self, msg): @@ -26,32 +24,30 @@ class CmdImportFile(CmdBase): def run(self): targets = [] - with DvcLock(self.is_locker, self.git): - output = self.parsed_args.output - self.verify_output(output, self.parsed_args.input) + output = self.parsed_args.output + self.verify_output(output, self.parsed_args.input) - for input in self.parsed_args.input: - if not os.path.isdir(input): - targets.append((input, output)) - else: - input_dir = os.path.basename(input) - for root, dirs, files in os.walk(input): - for file in files: - filename = os.path.join(root, file) + for input in self.parsed_args.input: + if not os.path.isdir(input): + targets.append((input, output)) + else: + input_dir = os.path.basename(input) + for root, dirs, files in os.walk(input): + for file in files: + filename = os.path.join(root, file) - rel = os.path.relpath(filename, input) - out = os.path.join(output, input_dir, rel) + rel = os.path.relpath(filename, input) + out = os.path.join(output, input_dir, rel) - out_dir = os.path.dirname(out) - if not os.path.exists(out_dir): - os.mkdir(out_dir) + out_dir = os.path.dirname(out) + if not os.path.exists(out_dir): + os.mkdir(out_dir) - targets.append((filename, out)) - pass - self.import_files(targets, self.parsed_args.lock, self.parsed_args.jobs) - message = 'DVC import files: {} -> {}'.format(str(self.parsed_args.input), output) - self.commit_if_needed(message) - pass + targets.append((filename, out)) + pass + self.import_files(targets, self.parsed_args.lock, self.parsed_args.jobs) + message = 'DVC import files: {} -> {}'.format(str(self.parsed_args.input), output) + self.commit_if_needed(message) @staticmethod def verify_output(output, input): diff --git a/dvc/command/init.py b/dvc/command/init.py index 44b807ffa..40bda25e5 100644 --- a/dvc/command/init.py +++ b/dvc/command/init.py @@ -62,6 +62,10 @@ ProjectName = def __init__(self, settings): super(CmdInit, self).__init__(settings) + @property + def is_locker(self): + return False + def get_not_existing_path(self, *args): path = Path(os.path.join(self.git.git_dir, *args)) if path.exists(): diff --git a/dvc/command/instance_create.py b/dvc/command/instance_create.py index ecd0a9412..6697fa769 100644 --- a/dvc/command/instance_create.py +++ b/dvc/command/instance_create.py @@ -1,5 +1,5 @@ from dvc.cloud.instance_manager import InstanceManager -from dvc.command.common.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase from dvc.exceptions import DvcException from dvc.logger import Logger @@ -9,21 +9,20 @@ class CmdInstanceCreate(CmdBase): super(CmdInstanceCreate, self).__init__(settings) def run(self): - with DvcLock(self.is_locker, self.git): - name = self.parsed_args.name - cloud = self.parsed_args.cloud or self.settings.config.cloud + name = self.parsed_args.name + cloud = self.parsed_args.cloud or self.settings.config.cloud - # print('NAME: {}'.format(name)) - # print('CLOUD: {}'.format(cloud)) + # print('NAME: {}'.format(name)) + # print('CLOUD: {}'.format(cloud)) - if not name: - Logger.error('Instance name is not defined') - return 1 + if not name: + Logger.error('Instance name is not defined') + return 1 - try: - InstanceManager().create(name, cloud, self.parsed_args, self.settings.config) - except DvcException as ex: - Logger.error('Instance creation error: {}'.format(ex)) - return 1 + try: + InstanceManager().create(name, cloud, self.parsed_args, self.settings.config) + except DvcException as ex: + Logger.error('Instance creation error: {}'.format(ex)) + return 1 - return 0 + return 0 diff --git a/dvc/command/lock.py b/dvc/command/lock.py index 4ac0b12e7..7a2b4ce2d 100644 --- a/dvc/command/lock.py +++ b/dvc/command/lock.py @@ -1,4 +1,4 @@ -from dvc.command.common.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase from dvc.logger import Logger from dvc.state_file import StateFile @@ -8,8 +8,7 @@ class CmdLock(CmdBase): super(CmdLock, self).__init__(settings) def run(self): - with DvcLock(self.is_locker, self.git): - return self.lock_files(self.parsed_args.files, not self.parsed_args.unlock) + return self.lock_files(self.parsed_args.files, not self.parsed_args.unlock) def lock_files(self, files, target): cmd = 'lock' if target else 'unlock' diff --git a/dvc/command/merge.py b/dvc/command/merge.py index edaf39310..9fa2c2cca 100644 --- a/dvc/command/merge.py +++ b/dvc/command/merge.py @@ -1,6 +1,6 @@ import os -from dvc.command.common.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase from dvc.config import ConfigI from dvc.logger import Logger from dvc.state_file import StateFile @@ -63,12 +63,11 @@ class CmdMerge(CmdBase): self.commit_if_needed(msg) def run(self): - with DvcLock(self.is_locker, self.git): - targets = self.collect_targets() - if not targets: - return 1 + targets = self.collect_targets() + if not targets: + return 1 - self.checkout_targets(targets) - self.print_info(targets) + self.checkout_targets(targets) + self.print_info(targets) - return 0 + return 0 diff --git a/dvc/command/repro.py b/dvc/command/repro.py index 433b5d6ab..7c85b32a4 100644 --- a/dvc/command/repro.py +++ b/dvc/command/repro.py @@ -1,7 +1,6 @@ import os import copy -from dvc.command.common.base import DvcLock from dvc.command.import_file import CmdImportFile from dvc.command.run import CmdRun from dvc.logger import Logger @@ -40,23 +39,21 @@ class CmdRepro(CmdRun): return [] def run(self): - with DvcLock(self.is_locker, self.git): - recursive = not self.parsed_args.single_item - targets = [] + recursive = not self.parsed_args.single_item + targets = [] - if self.parsed_args.target: - targets += self.parsed_args.target - else: - target = self.settings.config._config['Global'].get('Target', None) - if not target or len(target) == 0: - Logger.error('Reproduction target is not defined. ' + - 'Specify data file or set target by ' + - '`dvc config global.target target` command.') - return 1 - targets += [target] + if self.parsed_args.target: + targets += self.parsed_args.target + else: + target = self.settings.config._config['Global'].get('Target', None) + if not target or len(target) == 0: + Logger.error('Reproduction target is not defined. ' + + 'Specify data file or set target by ' + + '`dvc config global.target target` command.') + return 1 + targets += [target] - return self.repro_target(targets, recursive, self.parsed_args.force) - pass + return self.repro_target(targets, recursive, self.parsed_args.force) def repro_target(self, target, recursive, force): if not self.no_git_actions and not self.git.is_ready_to_go(): diff --git a/dvc/command/run.py b/dvc/command/run.py index bb4595ad1..679e2f5ac 100644 --- a/dvc/command/run.py +++ b/dvc/command/run.py @@ -1,6 +1,6 @@ import os -from dvc.command.common.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase from dvc.exceptions import DvcException from dvc.git_wrapper import GitWrapper from dvc.logger import Logger @@ -35,16 +35,14 @@ class CmdRun(CmdBase): return self._data_items_from_params(self.parsed_args.output, 'Output') def run(self): - with DvcLock(self.is_locker, self.git): - cmd = [self.parsed_args.command] + self.parsed_args.args - data_items_from_args, not_data_items_from_args = self.argv_files_by_type(cmd) - return self.run_and_commit_if_needed(cmd, - data_items_from_args, - not_data_items_from_args, - self.parsed_args.stdout, - self.parsed_args.stderr, - self.parsed_args.shell) - pass + cmd = [self.parsed_args.command] + self.parsed_args.args + data_items_from_args, not_data_items_from_args = self.argv_files_by_type(cmd) + return self.run_and_commit_if_needed(cmd, + data_items_from_args, + not_data_items_from_args, + self.parsed_args.stdout, + self.parsed_args.stderr, + self.parsed_args.shell) def run_and_commit_if_needed(self, command_args, data_items_from_args, not_data_items_from_args, stdout, stderr, shell, diff --git a/dvc/command/traverse.py b/dvc/command/traverse.py index 2d562a144..2d7df39ad 100644 --- a/dvc/command/traverse.py +++ b/dvc/command/traverse.py @@ -1,10 +1,11 @@ import os -from dvc.command.common.base import CmdBase, DvcLock +from dvc.command.common.base import CmdBase from dvc.exceptions import DvcException from dvc.logger import Logger from dvc.path.data_item import DataItemError + class TraverseError(DvcException): def __init__(self, msg): DvcException.__init__(self, 'Traverse file tree error: {}'.format(msg)) @@ -17,9 +18,8 @@ class Traverse(CmdBase): self._do_not_start_from_root = do_not_start_from_root def run(self): - with DvcLock(self.is_locker, self.git): - if not self._traverse_all(): - return 1 + if not self._traverse_all(): + return 1 return 0 From e753d7fc7977990182aa295b8040c4ff64f2d561 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sun, 19 Nov 2017 04:22:30 -0800 Subject: [PATCH 6/8] Generalize CmdDataBase class --- dvc/command/data_sync.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/dvc/command/data_sync.py b/dvc/command/data_sync.py index f8b188fc9..0098b3826 100644 --- a/dvc/command/data_sync.py +++ b/dvc/command/data_sync.py @@ -4,20 +4,23 @@ from dvc.logger import Logger import dvc.data_cloud as cloud -class CmdDataPull(CmdBase): - def __init__(self, settings): - super(CmdDataPull, self).__init__(settings) +class CmdDataBase(CmdBase): + def __init__(self, settings, cmd): + super(CmdDataBase, self).__init__(settings) + self.cmd = cmd def run(self): - self.cloud.pull(self.parsed_args.targets, self.parsed_args.jobs) + self.cmd(self.parsed_args.targets, self.parsed_args.jobs) -class CmdDataPush(CmdBase): +class CmdDataPull(CmdDataBase): def __init__(self, settings): - super(CmdDataPush, self).__init__(settings) + super(CmdDataPull, self).__init__(settings, self.cloud.pull) - def run(self): - self.cloud.push(self.parsed_args.targets, self.parsed_args.jobs) + +class CmdDataPush(CmdDataBase): + def __init__(self, settings): + super(CmdDataPush, self).__init__(settings, self.cloud.push) class CmdDataStatus(CmdBase): From 5bd7819a53d00207c876c52b8d057618e78e7ac1 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sun, 19 Nov 2017 04:26:58 -0800 Subject: [PATCH 7/8] Exctract Traverse class to command.common --- dvc/command/{ => common}/traverse.py | 0 dvc/command/gc.py | 2 +- dvc/command/remove.py | 2 +- dvc/command/show_pipeline.py | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename dvc/command/{ => common}/traverse.py (100%) diff --git a/dvc/command/traverse.py b/dvc/command/common/traverse.py similarity index 100% rename from dvc/command/traverse.py rename to dvc/command/common/traverse.py diff --git a/dvc/command/gc.py b/dvc/command/gc.py index 7f73e925d..ada0bac8e 100644 --- a/dvc/command/gc.py +++ b/dvc/command/gc.py @@ -1,6 +1,6 @@ import os -from dvc.command.traverse import Traverse +from dvc.command.common.traverse import Traverse from dvc.logger import Logger from dvc.path.data_item import DataItemError from dvc.config import ConfigI diff --git a/dvc/command/remove.py b/dvc/command/remove.py index 00ec8b058..a9f96027c 100644 --- a/dvc/command/remove.py +++ b/dvc/command/remove.py @@ -1,6 +1,6 @@ import os -from dvc.command.traverse import Traverse +from dvc.command.common.traverse import Traverse from dvc.logger import Logger diff --git a/dvc/command/show_pipeline.py b/dvc/command/show_pipeline.py index 1fe8c916c..be29d2c7c 100644 --- a/dvc/command/show_pipeline.py +++ b/dvc/command/show_pipeline.py @@ -1,7 +1,7 @@ import os import networkx as nx -from dvc.command.traverse import Traverse +from dvc.command.common.traverse import Traverse from dvc.logger import Logger from dvc.state_file import StateFile From d64390e9d02d0a08e47139d30d0668f11d07a978 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Sun, 19 Nov 2017 04:53:14 -0800 Subject: [PATCH 8/8] Handle errors when switching branches --- dvc/command/common/branch_changer.py | 12 ++++++++---- dvc/git_wrapper.py | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/dvc/command/common/branch_changer.py b/dvc/command/common/branch_changer.py index 819b7a33b..3980aeaf2 100644 --- a/dvc/command/common/branch_changer.py +++ b/dvc/command/common/branch_changer.py @@ -2,8 +2,8 @@ from dvc.command.common.common_error import CmdCommonError class BranchChangerError(CmdCommonError): - def __init__(self, msg): - super(CmdCommonError, self).__init__('DVC locker error: {}'.format(msg)) + def __init__(self, branch, msg): + super(CmdCommonError, self).__init__('Error in changing branch \'{}\': {}'.format(branch, msg)) class BranchChanger(object): @@ -19,9 +19,13 @@ class BranchChanger(object): def __enter__(self): if self.perform_action: - self.git.checkout(self.branch, self.create_new) + code, _, err = self.git.checkout(self.branch, self.create_new) + if code != 0: + raise BranchChangerError(self.branch, err) return self def __exit__(self, type, value, traceback): if self.perform_action: - self.git.checkout_previous() + code, _, err = self.git.checkout_previous() + if code != 0: + raise BranchChangerError(self.branch, err) diff --git a/dvc/git_wrapper.py b/dvc/git_wrapper.py index 02b5af2de..18ef1c7c0 100644 --- a/dvc/git_wrapper.py +++ b/dvc/git_wrapper.py @@ -321,13 +321,13 @@ class GitWrapper(GitWrapperI): prefix = '-b ' if create_new else '' Logger.debug('[dvc-git] Checkout {}{}'.format(prefix, commit_or_branch)) cmd = 'git checkout {}{}'.format(prefix, commit_or_branch) - Executor.exec_cmd(cmd.split()) + return Executor.exec_cmd(cmd.split()) @staticmethod def checkout_previous(): Logger.debug('[dvc-git] Checkout previous') cmd = 'git checkout -' - Executor.exec_cmd(cmd.split()) + return Executor.exec_cmd(cmd.split()) def separate_dependency_files_and_dirs(self, code_dependencies): code_files = []