From 1439f3be600473c290d4863cfa4eb37d01674d31 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Tue, 20 Oct 2015 23:57:56 +0200 Subject: [PATCH 1/3] give a final status into the log output, including exit code, fixes #58 --- borg/archiver.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index f6da5e1ea..7979b8680 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -1042,22 +1042,31 @@ def main(): # pragma: no cover setup_signal_handlers() archiver = Archiver() try: + msg = None exit_code = archiver.run(sys.argv[1:]) except Error as e: - archiver.print_error(e.get_message() + "\n%s" % traceback.format_exc()) + msg = e.get_message() + "\n%s" % traceback.format_exc() exit_code = e.exit_code except RemoteRepository.RPCError as e: - archiver.print_error('Error: Remote Exception.\n%s' % str(e)) + msg = 'Remote Exception.\n%s' % str(e) exit_code = 1 except Exception: - archiver.print_error('Error: Local Exception.\n%s' % traceback.format_exc()) + msg = 'Local Exception.\n%s' % traceback.format_exc() exit_code = 1 except KeyboardInterrupt: - archiver.print_error('Error: Keyboard interrupt.\n%s' % traceback.format_exc()) + msg = 'Keyboard interrupt.\n%s' % traceback.format_exc() exit_code = 1 - if exit_code: - archiver.print_error('Exiting with failure status due to previous errors') + if msg: + logger.error(msg) + if exit_code == 0: + logger.info('terminating with success status, rc %d' % exit_code) + else: + # TODO: + # this should differentiate between normal termination (there were warnings, but + # code reached the end of backup) and abrupt termination due to an error or exception. + logger.error('terminating with failure status due to previous errors, rc %d' % exit_code) sys.exit(exit_code) + if __name__ == '__main__': main() From 72c984891c964aecaf05c4f3432608b1cae95c6d Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 21 Oct 2015 01:11:51 +0200 Subject: [PATCH 2/3] refactor return codes, fixes #61 0 = success (logged as INFO) 1 = warning (logged as WARNING) 2 = (fatal and abrupt) error (logged as ERROR) please use the EXIT_(SUCCESS,WARNING,ERROR) constants from helpers module. --- borg/archiver.py | 41 ++++++++++++++++++++++------------------- borg/helpers.py | 22 ++++++++++++++++------ 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index 7979b8680..9ef15222c 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -19,7 +19,8 @@ from .helpers import Error, location_validator, format_time, format_file_size, \ format_file_mode, ExcludePattern, IncludePattern, exclude_path, adjust_patterns, to_localtime, timestamp, \ get_cache_dir, get_keys_dir, format_timedelta, prune_within, prune_split, \ Manifest, remove_surrogates, update_excludes, format_archive, check_extension_modules, Statistics, \ - is_cachedir, bigint_to_int, ChunkerParams, CompressionSpec, have_cython + is_cachedir, bigint_to_int, ChunkerParams, CompressionSpec, have_cython, \ + EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR from .logger import create_logger, setup_logging logger = create_logger() if have_cython(): @@ -37,7 +38,7 @@ has_lchflags = hasattr(os, 'lchflags') class Archiver: def __init__(self): - self.exit_code = 0 + self.exit_code = EXIT_SUCCESS def open_repository(self, location, create=False, exclusive=False): if location.proto == 'ssh': @@ -49,7 +50,7 @@ class Archiver: def print_error(self, msg, *args): msg = args and msg % args or msg - self.exit_code = 1 + self.exit_code = EXIT_WARNING # we do not terminate here, so it is a warning logger.error('borg: ' + msg) def print_verbose(self, msg, *args, **kw): @@ -92,18 +93,18 @@ Type "Yes I am sure" if you understand this and want to continue.\n""") if repository.check(repair=args.repair): logger.info('Repository check complete, no problems found.') else: - return 1 + return EXIT_WARNING if not args.repo_only and not ArchiveChecker().check( repository, repair=args.repair, archive=args.repository.archive, last=args.last): - return 1 - return 0 + return EXIT_WARNING + return EXIT_SUCCESS def do_change_passphrase(self, args): """Change repository key file passphrase""" repository = self.open_repository(args.repository) manifest, key = Manifest.load(repository) key.change_passphrase() - return 0 + return EXIT_SUCCESS def do_create(self, args): """Create new archive""" @@ -326,7 +327,7 @@ Type "Yes I am sure" if you understand this and want to continue.\n""") print("""Type "YES" if you understand this and want to continue.\n""", file=sys.stderr) # XXX: prompt may end up on stdout, but we'll assume that input() does the right thing if input('Do you want to continue? ') != 'YES': - self.exit_code = 1 + self.exit_code = EXIT_ERROR return self.exit_code repository.destroy() logger.info("Repository deleted.") @@ -358,7 +359,7 @@ Type "Yes I am sure" if you understand this and want to continue.\n""") operations.mount(args.mountpoint, args.options, args.foreground) except RuntimeError: # Relevant error message already printed to stderr by fuse - self.exit_code = 1 + self.exit_code = EXIT_ERROR return self.exit_code def do_list(self, args): @@ -431,7 +432,7 @@ Type "Yes I am sure" if you understand this and want to continue.\n""") if args.hourly + args.daily + args.weekly + args.monthly + args.yearly == 0 and args.within is None: self.print_error('At least one of the "within", "keep-hourly", "keep-daily", "keep-weekly", ' '"keep-monthly" or "keep-yearly" settings must be specified') - return 1 + return EXIT_ERROR if args.prefix: archives = [archive for archive in archives if archive.name.startswith(args.prefix)] keep = [] @@ -1049,22 +1050,24 @@ def main(): # pragma: no cover exit_code = e.exit_code except RemoteRepository.RPCError as e: msg = 'Remote Exception.\n%s' % str(e) - exit_code = 1 + exit_code = EXIT_ERROR except Exception: msg = 'Local Exception.\n%s' % traceback.format_exc() - exit_code = 1 + exit_code = EXIT_ERROR except KeyboardInterrupt: msg = 'Keyboard interrupt.\n%s' % traceback.format_exc() - exit_code = 1 + exit_code = EXIT_ERROR if msg: logger.error(msg) - if exit_code == 0: - logger.info('terminating with success status, rc %d' % exit_code) + exit_msg = 'terminating with %s status, rc %d' + if exit_code == EXIT_SUCCESS: + logger.info(exit_msg % ('success', exit_code)) + elif exit_code == EXIT_WARNING: + logger.warning(exit_msg % ('warning', exit_code)) + elif exit_code == EXIT_ERROR: + logger.error(exit_msg % ('error', exit_code)) else: - # TODO: - # this should differentiate between normal termination (there were warnings, but - # code reached the end of backup) and abrupt termination due to an error or exception. - logger.error('terminating with failure status due to previous errors, rc %d' % exit_code) + logger.error(exit_msg % ('abnormal', exit_code)) sys.exit(exit_code) diff --git a/borg/helpers.py b/borg/helpers.py index 9ba8c8ed7..1a36a90d0 100644 --- a/borg/helpers.py +++ b/borg/helpers.py @@ -44,13 +44,27 @@ if have_cython(): import msgpack +# return codes returned by borg command +# when borg is killed by signal N, rc = 128 + N +EXIT_SUCCESS = 0 # everything done, no problems +EXIT_WARNING = 1 # reached normal end of operation, but there were issues +EXIT_ERROR = 2 # terminated abruptly, did not reach end of operation + + class Error(Exception): """Error base class""" - exit_code = 1 + # if we raise such an Error and it is only catched by the uppermost + # exception handler (that exits short after with the given exit_code), + # it is always a (fatal and abrupt) EXIT_ERROR, never just a warning. + exit_code = EXIT_ERROR def get_message(self): - return 'Error: ' + type(self).__doc__.format(*self.args) + return type(self).__doc__.format(*self.args) + + +class IntegrityError(Error): + """Data integrity error""" class ExtensionModuleError(Error): @@ -487,10 +501,6 @@ def format_archive(archive): return '%-36s %s' % (archive.name, to_localtime(archive.ts).strftime('%c')) -class IntegrityError(Error): - """Data integrity error""" - - def memoize(function): cache = {} From 5bcd4835e6042daf160f13c6b4744a5c779dab2d Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 22 Oct 2015 00:45:29 +0200 Subject: [PATCH 3/3] add tests for return codes --- borg/testsuite/archiver.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/borg/testsuite/archiver.py b/borg/testsuite/archiver.py index 7779c6fe7..f0adf2cba 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -20,7 +20,7 @@ from ..archive import Archive, ChunkBuffer, CHUNK_MAX_EXP from ..archiver import Archiver from ..cache import Cache from ..crypto import bytes_to_long, num_aes_blocks -from ..helpers import Manifest +from ..helpers import Manifest, EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR from ..remote import RemoteRepository, PathNotAllowed from ..repository import Repository from . import BaseTestCase @@ -122,6 +122,24 @@ def cmd(request): return exec_fn +def test_return_codes(cmd, tmpdir): + repo = tmpdir.mkdir('repo') + input = tmpdir.mkdir('input') + output = tmpdir.mkdir('output') + input.join('test_file').write('content') + rc, out = cmd('init', '%s' % str(repo)) + assert rc == EXIT_SUCCESS + rc, out = cmd('create', '%s::archive' % repo, str(input)) + assert rc == EXIT_SUCCESS + with changedir(str(output)): + rc, out = cmd('extract', '%s::archive' % repo) + assert rc == EXIT_SUCCESS + rc, out = cmd('extract', '%s::archive' % repo, 'does/not/match') + assert rc == EXIT_WARNING # pattern did not match + rc, out = cmd('create', '%s::archive' % repo, str(input)) + assert rc == EXIT_ERROR # duplicate archive name + + class ArchiverTestCaseBase(BaseTestCase): EXE = None # python source based FORK_DEFAULT = False