From fe32002a7f7e14e7d30d604681203858e6c329e0 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sun, 2 Dec 2018 07:39:37 +0200 Subject: [PATCH] dvc: handle KeyboardInterrupt Fixes #1389 Signed-off-by: Ruslan Kuprieiev --- dvc/logger.py | 52 +++++++++++++++++++++++++------------------- dvc/main.py | 5 ++++- tests/test_logger.py | 48 ++++++++++++++++++++++++++++++++++++++++ tests/test_run.py | 7 ++++++ 4 files changed, 89 insertions(+), 23 deletions(-) diff --git a/dvc/logger.py b/dvc/logger.py index b0dc3e356..caa79132a 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -106,21 +106,26 @@ class Logger(object): @staticmethod def parse_exc(exc, tb=None): str_tb = tb if tb else None - str_exc = ': {}'.format(str(exc)) if exc else "" + str_exc = str(exc) if exc else "" + l_str_exc = [] + + if len(str_exc) != 0: + l_str_exc.append(str_exc) if exc and hasattr(exc, 'cause') and exc.cause: cause_tb = exc.cause_tb if hasattr(exc, 'cause_tb') else None - cause_str_exc, cause_str_tb = Logger.parse_exc(exc.cause, cause_tb) + l_cause_str_exc, cause_str_tb = Logger.parse_exc(exc.cause, + cause_tb) str_tb = cause_str_tb - str_exc = '{}{}'.format(str_exc, cause_str_exc) + l_str_exc += l_cause_str_exc - return (str_exc, str_tb) + return (l_str_exc, str_tb) @staticmethod def _prefix(msg, typ): color = Logger.LEVEL_COLOR_MAP.get(typ.lower(), '') - return Logger.colorize('{}: '.format(msg), color) + return Logger.colorize('{}'.format(msg), color) @staticmethod def error_prefix(): @@ -141,43 +146,42 @@ class Logger(object): func(msg) @staticmethod - def _error_exc(exc): - if exc is None: - return + def _with_exc(func, prefix, msg, suffix="", exc=None): + l_str_exc, str_tb = Logger.parse_exc(exc) - if Logger.logger().getEffectiveLevel() != logging.DEBUG: - return + if exc is not None and Logger.is_verbose(): + str_tb = str_tb if str_tb else traceback.format_exc() + Logger._with_progress(Logger.logger().error, str_tb) - prefix = Logger.error_prefix() - str_exc, str_tb = Logger.parse_exc(exc) - str_tb = str_tb if str_tb else traceback.format_exc() - Logger._with_progress(Logger.logger().error, prefix + str_tb) + l_msg = [prefix] + if msg is not None and len(msg) != 0: + l_msg.append(msg) + l_msg += l_str_exc - @staticmethod - def _with_exc(func, msg, suffix="", exc=None): - Logger._error_exc(exc) - msg = msg + Logger.parse_exc(exc)[0] + suffix - Logger._with_progress(func, msg) + Logger._with_progress(func, ': '.join(l_msg) + suffix) @staticmethod def error(msg, exc=None): chat = "\n\nHaving any troubles? Hit us up at dvc.org/support, " \ "we are always happy to help!" Logger._with_exc(Logger.logger().error, - Logger.error_prefix() + msg, + Logger.error_prefix(), + msg, suffix=chat, exc=exc) @classmethod def warn(cls, msg, exc=None): cls._with_exc(cls.logger().warning, - cls.warning_prefix() + msg, + cls.warning_prefix(), + msg, exc=exc) @classmethod def debug(cls, msg, exc=None): cls._with_exc(cls.logger().debug, - cls.debug_prefix() + msg, + cls.debug_prefix(), + msg, exc=exc) @staticmethod @@ -188,6 +192,10 @@ class Logger(object): def is_quiet(): return Logger.logger().level == logging.CRITICAL + @staticmethod + def is_verbose(): + return Logger.logger().getEffectiveLevel() == logging.DEBUG + @classmethod def box(cls, msg, border_color=''): if cls.is_quiet(): diff --git a/dvc/main.py b/dvc/main.py index c95867770..ab1aa96ee 100644 --- a/dvc/main.py +++ b/dvc/main.py @@ -20,8 +20,11 @@ def main(argv=None): cmd = args.func(args) ret = cmd.run_cmd() + except KeyboardInterrupt as ex: + Logger.error("Interrupted by the user", ex) + ret = 252 except NotDvcProjectError as ex: - Logger.error(str(ex)) + Logger.error("", ex) ret = 253 except DvcParserError: ret = 254 diff --git a/tests/test_logger.py b/tests/test_logger.py index 27755c86b..a50b60a75 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -70,6 +70,54 @@ class TestLogger(TestDvc): mock_stdout.getvalue()) +class TestLoggerException(TestDvc): + @patch('sys.stderr', new_callable=StringIO) + @patch('sys.stdout', new_callable=StringIO) + def test_empty_err_msg(self, mock_stdout, mock_stderr): + from dvc.exceptions import DvcException + + Logger.init() + exc_msg = 'msg' + Logger.error("", exc=DvcException(exc_msg)) + + self.assertEqual('', mock_stdout.getvalue()) + self.assertEqual('Error: {}\n\nHaving any troubles? ' + 'Hit us up at dvc.org/support, we ' + 'are always happy to help!\n'.format(exc_msg), + mock_stderr.getvalue()) + + @patch('sys.stderr', new_callable=StringIO) + @patch('sys.stdout', new_callable=StringIO) + def test_empty_exc_msg(self, mock_stdout, mock_stderr): + from dvc.exceptions import DvcException + + Logger.init() + err_msg = 'msg' + Logger.error(err_msg, exc=DvcException("")) + + self.assertEqual('', mock_stdout.getvalue()) + self.assertEqual('Error: {}\n\nHaving any troubles? ' + 'Hit us up at dvc.org/support, we ' + 'are always happy to help!\n'.format(err_msg), + mock_stderr.getvalue()) + + @patch('sys.stderr', new_callable=StringIO) + @patch('sys.stdout', new_callable=StringIO) + def test_msg_and_exc(self, mock_stdout, mock_stderr): + from dvc.exceptions import DvcException + + Logger.init() + err_msg = 'error' + exc_msg = 'exception' + Logger.error(err_msg, exc=DvcException(exc_msg)) + + self.assertEqual('', mock_stdout.getvalue()) + self.assertEqual('Error: {}: {}\n\nHaving any troubles? ' + 'Hit us up at dvc.org/support, we are always ' + 'happy to help!\n'.format(err_msg, exc_msg), + mock_stderr.getvalue()) + + class TestLoggerQuiet(TestDvc): def setUp(self): super(TestLoggerQuiet, self).setUp() diff --git a/tests/test_run.py b/tests/test_run.py index 82c9da066..2e5f0a932 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -1,5 +1,7 @@ import os +import mock import filecmp +import subprocess from dvc.main import main from dvc.utils import file_md5 @@ -173,3 +175,8 @@ class TestCmdRun(TestDvc): with open(log, 'r') as fobj: self.assertEqual(fobj.read(), arg) + + @mock.patch.object(subprocess, 'Popen', side_effect=KeyboardInterrupt) + def test_keyboard_interrupt(self, mock_popen): + ret = main(['run', 'mycmd']) + self.assertEqual(ret, 252)