From 8597a526d78705c411fcaa8816712bd3c5b3ce00 Mon Sep 17 00:00:00 2001 From: Brett Smith Date: Thu, 30 Jul 2020 15:53:31 -0400 Subject: [PATCH] cliutil: Use semi-standardized BSD exit codes. --- conservancy_beancount/cliutil.py | 73 +++++++++++-------- conservancy_beancount/reports/accrual.py | 12 ++- conservancy_beancount/reports/fund.py | 10 ++- conservancy_beancount/reports/ledger.py | 10 ++- .../tools/opening_balances.py | 8 +- setup.py | 2 +- tests/test_cliutil.py | 4 +- tests/test_reports_accrual.py | 13 ++-- tests/test_reports_fund.py | 2 +- tests/test_reports_ledger.py | 2 +- 10 files changed, 82 insertions(+), 54 deletions(-) diff --git a/conservancy_beancount/cliutil.py b/conservancy_beancount/cliutil.py index 22167f9..ea7768e 100644 --- a/conservancy_beancount/cliutil.py +++ b/conservancy_beancount/cliutil.py @@ -32,6 +32,8 @@ import types from pathlib import Path +import rt.exceptions as rt_error + from . import data from . import filters from . import rtutil @@ -61,38 +63,67 @@ STDSTREAM_PATH = Path('-') VERSION = pkg_resources.require(PKGNAME)[0].version class ExceptHook: - def __init__(self, - logger: Optional[logging.Logger]=None, - default_exitcode: int=3, - ) -> None: + def __init__(self, logger: Optional[logging.Logger]=None) -> None: if logger is None: logger = logging.getLogger() self.logger = logger - self.default_exitcode = default_exitcode def __call__(self, exc_type: Type[BaseException], exc_value: BaseException, exc_tb: types.TracebackType, ) -> NoReturn: - exitcode = self.default_exitcode + error_type = type(exc_value).__name__ + msg = ": ".join(str(arg) for arg in exc_value.args) if isinstance(exc_value, KeyboardInterrupt): signal.signal(signal.SIGINT, signal.SIG_DFL) os.kill(0, signal.SIGINT) signal.pause() + elif isinstance(exc_value, ( + rt_error.AuthorizationError, + rt_error.NotAllowed, + )): + exitcode = os.EX_NOPERM + error_type = "RT access denied" + elif isinstance(exc_value, rt_error.ConnectionError): + exitcode = os.EX_TEMPFAIL + error_type = "RT connection error" + elif isinstance(exc_value, rt_error.RtError): + exitcode = os.EX_UNAVAILABLE + error_type = f"RT {error_type}" elif isinstance(exc_value, OSError): - exitcode += 1 - msg = "I/O error: {e.filename}: {e.strerror}".format(e=exc_value) + if exc_value.filename is None: + exitcode = os.EX_OSERR + error_type = "OS error" + msg = exc_value.strerror + else: + # There are more specific exit codes for input problems vs. + # output problems, but without knowing how the file was + # intended to be used, we can't use them. + exitcode = os.EX_IOERR + error_type = "I/O error" + msg = f"{exc_value.filename}: {exc_value.strerror}" else: - parts = [type(exc_value).__name__, *exc_value.args] - msg = "internal " + ": ".join(parts) - self.logger.critical(msg) + exitcode = os.EX_SOFTWARE + error_type = f"internal {error_type}" + self.logger.critical("%s%s%s", error_type, ": " if msg else "", msg) self.logger.debug( ''.join(traceback.format_exception(exc_type, exc_value, exc_tb)), ) raise SystemExit(exitcode) +class ExitCode(enum.IntEnum): + # BSD exit codes commonly used + NoConfiguration = os.EX_CONFIG + NoConfig = NoConfiguration + NoDataFiltered = os.EX_DATAERR + NoDataLoaded = os.EX_NOINPUT + + # Our own exit codes, working down from that range + BeancountErrors = 63 + + class InfoAction(argparse.Action): def __call__(self, parser: argparse.ArgumentParser, @@ -132,26 +163,6 @@ class LogLevel(enum.IntEnum): yield level.name.lower() -class ReturnFlag(enum.IntFlag): - """Common return codes for tools - - Tools should combine these flags to report different errors, and then use - ReturnFlag.returncode(flags) to report their final exit status code. - - Values 1, 2, 4, and 8 should be reserved for this class to be shared across - all tools. Flags 16, 32, and 64 are available for tools to report their own - specific errors. - """ - LOAD_ERRORS = 1 - NOTHING_TO_REPORT = 2 - _RESERVED4 = 4 - _RESERVED8 = 8 - - @classmethod - def returncode(cls, flags: int) -> int: - return 0 if flags == 0 else 16 + flags - - class SearchTerm(NamedTuple): """NamedTuple representing a user's metadata filter diff --git a/conservancy_beancount/reports/accrual.py b/conservancy_beancount/reports/accrual.py index 074b831..bb9ebfa 100644 --- a/conservancy_beancount/reports/accrual.py +++ b/conservancy_beancount/reports/accrual.py @@ -709,20 +709,24 @@ def main(arglist: Optional[Sequence[str]]=None, books_loader = config.books_loader() if books_loader is None: entries, load_errors, _ = books.Loader.load_none(config.config_file_path()) + returncode = cliutil.ExitCode.NoConfiguration else: load_since = None if args.report_type == ReportType.AGING else args.since entries, load_errors, _ = books_loader.load_all(load_since) + if load_errors: + returncode = cliutil.ExitCode.BeancountErrors + elif not entries: + returncode = cliutil.ExitCode.NoDataLoaded filters.remove_opening_balance_txn(entries) for error in load_errors: bc_printer.print_error(error, file=stderr) - returncode |= cliutil.ReturnFlag.LOAD_ERRORS postings = list(filter_search( data.Posting.from_entries(entries), args.search_terms, )) if not postings: logger.warning("no matching entries found to report") - returncode |= cliutil.ReturnFlag.NOTHING_TO_REPORT + returncode = returncode or cliutil.ExitCode.NoDataFiltered # groups is a mapping of metadata value strings to AccrualPostings. # The keys are basically arbitrary, the report classes don't rely on them, # but they do help symbolize what's being grouped. @@ -776,10 +780,10 @@ def main(arglist: Optional[Sequence[str]]=None, report = BalanceReport(out_file) if report is None: - returncode |= 16 + returncode = cliutil.ExitCode.NoConfiguration else: report.run(groups) - return cliutil.ReturnFlag.returncode(returncode) + return returncode entry_point = cliutil.make_entry_point(__name__, PROGNAME) diff --git a/conservancy_beancount/reports/fund.py b/conservancy_beancount/reports/fund.py index 4affa81..bee58fe 100644 --- a/conservancy_beancount/reports/fund.py +++ b/conservancy_beancount/reports/fund.py @@ -362,11 +362,15 @@ def main(arglist: Optional[Sequence[str]]=None, books_loader = config.books_loader() if books_loader is None: entries, load_errors, _ = books.Loader.load_none(config.config_file_path()) + returncode = cliutil.ExitCode.NoConfiguration else: entries, load_errors, _ = books_loader.load_fy_range(args.start_date, args.stop_date) + if load_errors: + returncode = cliutil.ExitCode.BeancountErrors + elif not entries: + returncode = cliutil.ExitCode.NoDataLoaded for error in load_errors: bc_printer.print_error(error, file=stderr) - returncode |= cliutil.ReturnFlag.LOAD_ERRORS postings = ( post @@ -387,7 +391,7 @@ def main(arglist: Optional[Sequence[str]]=None, ) if not fund_map: logger.warning("no matching postings found to report") - returncode |= cliutil.ReturnFlag.NOTHING_TO_REPORT + returncode = returncode or cliutil.ExitCode.NoDataFiltered elif args.report_type is ReportType.TEXT: out_file = cliutil.text_output(args.output_file, stdout) report = TextReport(args.start_date, args.stop_date, out_file) @@ -404,7 +408,7 @@ def main(arglist: Optional[Sequence[str]]=None, logger.info("Writing report to %s", args.output_file) ods_file = cliutil.bytes_output(args.output_file, stdout) ods_report.save_file(ods_file) - return cliutil.ReturnFlag.returncode(returncode) + return returncode entry_point = cliutil.make_entry_point(__name__, PROGNAME) diff --git a/conservancy_beancount/reports/ledger.py b/conservancy_beancount/reports/ledger.py index 9929eaf..279a958 100644 --- a/conservancy_beancount/reports/ledger.py +++ b/conservancy_beancount/reports/ledger.py @@ -770,11 +770,15 @@ def main(arglist: Optional[Sequence[str]]=None, books_loader = config.books_loader() if books_loader is None: entries, load_errors, options = books.Loader.load_none(config.config_file_path()) + returncode = cliutil.ExitCode.NoConfiguration else: entries, load_errors, options = books_loader.load_fy_range(args.start_date, args.stop_date) + if load_errors: + returncode = cliutil.ExitCode.BeancountErrors + elif not entries: + returncode = cliutil.ExitCode.NoDataLoaded for error in load_errors: bc_printer.print_error(error, file=stderr) - returncode |= cliutil.ReturnFlag.LOAD_ERRORS data.Account.load_from_books(entries, options) postings = data.Posting.from_entries(entries) @@ -813,7 +817,7 @@ def main(arglist: Optional[Sequence[str]]=None, report.write(postings) if not any(report.account_groups.values()): logger.warning("no matching postings found to report") - returncode |= cliutil.ReturnFlag.NOTHING_TO_REPORT + returncode = returncode or cliutil.ExitCode.NoDataFiltered if args.output_file is None: out_dir_path = config.repository_path() or Path() @@ -825,7 +829,7 @@ def main(arglist: Optional[Sequence[str]]=None, logger.info("Writing report to %s", args.output_file) ods_file = cliutil.bytes_output(args.output_file, stdout) report.save_file(ods_file) - return cliutil.ReturnFlag.returncode(returncode) + return returncode entry_point = cliutil.make_entry_point(__name__, PROGNAME) diff --git a/conservancy_beancount/tools/opening_balances.py b/conservancy_beancount/tools/opening_balances.py index 2248083..d9baa2e 100644 --- a/conservancy_beancount/tools/opening_balances.py +++ b/conservancy_beancount/tools/opening_balances.py @@ -191,11 +191,15 @@ def main(arglist: Optional[Sequence[str]]=None, books_loader = config.books_loader() if books_loader is None: entries, load_errors, _ = books.Loader.load_none(config.config_file_path()) + returncode = cliutil.ExitCode.NoConfiguration else: entries, load_errors, _ = books_loader.load_fy_range(0, args.as_of_date) + if load_errors: + returncode = cliutil.ExitCode.BeancountErrors + elif not entries: + returncode = cliutil.ExitCode.NoDataLoaded for error in load_errors: bc_printer.print_error(error, file=stderr) - returncode |= cliutil.ReturnFlag.LOAD_ERRORS inventories: Mapping[AccountWithFund, Inventory] = collections.defaultdict(Inventory) for post in Posting.from_entries(entries): @@ -247,7 +251,7 @@ def main(arglist: Optional[Sequence[str]]=None, dcontext = bc_dcontext.DisplayContext() dcontext.set_commas(True) bc_printer.print_entry(opening, dcontext, file=stdout) - return cliutil.ReturnFlag.returncode(returncode) + return returncode entry_point = cliutil.make_entry_point(__name__, PROGNAME) diff --git a/setup.py b/setup.py index f09adeb..e4743cc 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ from setuptools import setup setup( name='conservancy_beancount', description="Plugin, library, and reports for reading Conservancy's books", - version='1.6.3', + version='1.6.4', author='Software Freedom Conservancy', author_email='info@sfconservancy.org', license='GNU AGPLv3+', diff --git a/tests/test_cliutil.py b/tests/test_cliutil.py index 5a558d8..67b6bea 100644 --- a/tests/test_cliutil.py +++ b/tests/test_cliutil.py @@ -153,7 +153,7 @@ def test_excepthook_oserror(errnum, caplog): error = OSError(errnum, os.strerror(errnum), 'TestFilename') with pytest.raises(SystemExit) as exc_check: cliutil.ExceptHook()(type(error), error, None) - assert exc_check.value.args[0] == 4 + assert exc_check.value.args[0] == os.EX_IOERR assert caplog.records for log in caplog.records: assert log.levelname == 'CRITICAL' @@ -168,7 +168,7 @@ def test_excepthook_bug(exc_type, caplog): error = exc_type("test message") with pytest.raises(SystemExit) as exc_check: cliutil.ExceptHook()(exc_type, error, None) - assert exc_check.value.args[0] == 3 + assert exc_check.value.args[0] == os.EX_SOFTWARE assert caplog.records for log in caplog.records: assert log.levelname == 'CRITICAL' diff --git a/tests/test_reports_accrual.py b/tests/test_reports_accrual.py index c477e06..1df5f90 100644 --- a/tests/test_reports_accrual.py +++ b/tests/test_reports_accrual.py @@ -684,10 +684,11 @@ def run_main(arglist, config=None, out_type=io.StringIO): errors.seek(0) return retcode, output, errors -def check_main_fails(arglist, config, error_flags): +def check_main_fails(arglist, config, expect_retcode): + if not isinstance(expect_retcode, int): + expect_retcode = cliutil.ExitCode[expect_retcode] retcode, output, errors = run_main(arglist, config) - assert retcode > 16 - assert (retcode - 16) & error_flags + assert retcode == expect_retcode assert not output.getvalue() return errors @@ -775,7 +776,7 @@ def test_main_aging_report(arglist): check_aging_ods(output, datetime.date.today(), recv_rows, pay_rows) def test_main_no_books(): - errors = check_main_fails([], testutil.TestConfig(), 1 | 2) + errors = check_main_fails([], testutil.TestConfig(), 'NoConfiguration') testutil.check_lines_match(iter(errors), [ r':[01]: +no books to load in configuration\b', ]) @@ -786,7 +787,7 @@ def test_main_no_books(): ['-t', 'balance', 'entity=NonExistent'], ]) def test_main_no_matches(arglist, caplog): - check_main_fails(arglist, None, 2) + check_main_fails(arglist, None, 'NoDataFiltered') testutil.check_logs_match(caplog, [ ('WARNING', 'no matching entries found to report'), ]) @@ -795,7 +796,7 @@ def test_main_no_rt(caplog): config = testutil.TestConfig( books_path=testutil.test_path('books/accruals.beancount'), ) - check_main_fails(['-t', 'out'], config, 16) + check_main_fails(['-t', 'out'], config, 'NoConfiguration') testutil.check_logs_match(caplog, [ ('ERROR', 'unable to generate outgoing report: RT client is required'), ]) diff --git a/tests/test_reports_fund.py b/tests/test_reports_fund.py index ff2beeb..298ec57 100644 --- a/tests/test_reports_fund.py +++ b/tests/test_reports_fund.py @@ -280,5 +280,5 @@ def test_ods_report(start_date, stop_date): def test_main_no_postings(caplog): retcode, output, errors = run_main(io.StringIO, ['NonexistentProject']) - assert retcode == 18 + assert retcode == 65 assert any(log.levelname == 'WARNING' for log in caplog.records) diff --git a/tests/test_reports_ledger.py b/tests/test_reports_ledger.py index a3bf1e9..10e5732 100644 --- a/tests/test_reports_ledger.py +++ b/tests/test_reports_ledger.py @@ -557,5 +557,5 @@ def test_main_invalid_account(caplog, arg): def test_main_no_postings(caplog): retcode, output, errors = run_main(['NonexistentProject']) - assert retcode == 18 + assert retcode == 65 assert any(log.levelname == 'WARNING' for log in caplog.records)