From 32a076c039264e940c7575b05f1271a22c327832 Mon Sep 17 00:00:00 2001 From: "James R. Barlow" Date: Mon, 20 May 2019 18:01:17 -0700 Subject: [PATCH] Refactor validation and exceptions CLI now tracks check_options exceptions. API now works more like an API, without an exception handler, because the caller should provide one. --- src/ocrmypdf/__main__.py | 29 ++++++++++++--- src/ocrmypdf/_sync.py | 10 +---- src/ocrmypdf/_validation.py | 74 ++++++++++++------------------------- src/ocrmypdf/api.py | 6 +-- tests/test_unpaper.py | 7 ++-- 5 files changed, 54 insertions(+), 72 deletions(-) diff --git a/src/ocrmypdf/__main__.py b/src/ocrmypdf/__main__.py index 7b08f05e..b821c858 100755 --- a/src/ocrmypdf/__main__.py +++ b/src/ocrmypdf/__main__.py @@ -20,13 +20,16 @@ import logging import os import sys +from . import __version__ from .cli import parser -from .api import configure_logging, run as api_run -from ._validation import check_closed_streams -from .exceptions import ExitCode +from .api import configure_logging +from ._jobcontext import make_logger +from ._sync import run_pipeline +from ._validation import check_closed_streams, check_options +from .exceptions import ExitCode, BadArgsError, MissingDependencyError -def run(args=None): +def main(args=None): options = parser.parse_args(args=args) if not check_closed_streams(options): @@ -41,9 +44,23 @@ def run(args=None): if options.quiet: verbosity = -1 configure_logging(verbosity, manage_root_logger=True) - result = api_run(options=options) + log = make_logger('ocrmypdf') + log.debug('ocrmypdf ' + __version__) + try: + check_options(options) + except ValueError as e: + log.error(e) + return ExitCode.bad_args + except BadArgsError as e: + log.error(e) + return e.exit_code + except MissingDependencyError as e: + log.error(e) + return ExitCode.missing_dependency + + result = run_pipeline(options=options) return result if __name__ == '__main__': - sys.exit(run()) + sys.exit(main()) diff --git a/src/ocrmypdf/_sync.py b/src/ocrmypdf/_sync.py index fd98e63b..6a6aa2c5 100644 --- a/src/ocrmypdf/_sync.py +++ b/src/ocrmypdf/_sync.py @@ -28,7 +28,7 @@ from tempfile import mkdtemp from tqdm import tqdm -from . import VERSION +from . import __version__ from ._jobcontext import PDFContext, cleanup_working_files, make_logger from ._pipeline import ( convert_to_pdfa, @@ -237,12 +237,6 @@ def exec_concurrent(context): def run_pipeline(options): log = make_logger(options, __name__) - log.debug('ocrmypdf ' + VERSION) - - result = check_options(options) - if result != ExitCode.ok: - return result - check_dependency_versions(options) # Any changes to options will not take effect for options that are already # bound to function parameters in the pipeline. (For example @@ -256,8 +250,6 @@ def run_pipeline(options): # variable, but harmless to set if ignored. os.environ.setdefault('OMP_THREAD_LIMIT', '1') - check_environ(options) - work_folder = mkdtemp(prefix="com.github.ocrmypdf.") atexit.register(cleanup_working_files, work_folder, options) diff --git a/src/ocrmypdf/_validation.py b/src/ocrmypdf/_validation.py index fb1be0d1..07a8c481 100644 --- a/src/ocrmypdf/_validation.py +++ b/src/ocrmypdf/_validation.py @@ -20,7 +20,6 @@ import logging import os import sys -import textwrap from pathlib import Path import PIL @@ -28,7 +27,6 @@ import PIL from ._unicodefun import verify_python3_env from .exceptions import ( BadArgsError, - ExitCode, InputFileError, MissingDependencyError, OutputFileAccessError, @@ -243,26 +241,17 @@ def check_options_pillow(options): def check_options(options): - try: - check_options_languages(options) - check_options_metadata(options) - check_options_output(options) - check_options_sidecar(options) - check_options_preprocessing(options) - check_options_ocr_behavior(options) - check_options_optimizing(options) - check_options_advanced(options) - check_options_pillow(options) - return ExitCode.ok - except ValueError as e: - log.error(e) - return ExitCode.bad_args - except BadArgsError as e: - log.error(e) - return e.exit_code - except MissingDependencyError as e: - log.error(e) - return ExitCode.missing_dependency + check_options_languages(options) + check_options_metadata(options) + check_options_output(options) + check_options_sidecar(options) + check_options_preprocessing(options) + check_options_ocr_behavior(options) + check_options_optimizing(options) + check_options_advanced(options) + check_options_pillow(options) + check_dependency_versions(options) + check_environ(options) def check_closed_streams(options): @@ -334,11 +323,8 @@ def check_environ(options): for k in old_envvars: if k in os.environ: log.warning( - textwrap.dedent( - f"""\ - OCRmyPDF no longer uses the environment variable {k}. - Change PATH to select alternate programs.""" - ) + "OCRmyPDF no longer uses the environment variable {k}." + "Change PATH to select alternate programs." ) @@ -358,8 +344,7 @@ def create_input_file(options, work_folder): re_symlink(options.input_file, target) return target except FileNotFoundError: - log.error("File not found - %s", options.input_file) - raise InputFileError() + raise InputFileError(f"File not found - {options.input_file}") def check_input_file(options, start_input_file): @@ -374,27 +359,21 @@ def check_input_file(options, start_input_file): try: re_symlink(options.input_file, start_input_file) except FileNotFoundError: - log.error("File not found - %s", options.input_file) - raise InputFileError() + raise InputFileError(f"File not found - {options.input_file}") def check_requested_output_file(options): if options.output_file == '-': if sys.stdout.isatty(): - log.error( - textwrap.dedent( - """\ - Output was set to stdout '-' but it looks like stdout - is connected to a terminal. Please redirect stdout to a - file.""" - ) + raise BadArgsError( + "Output was set to stdout '-' but it looks like stdout " + "is connected to a terminal. Please redirect stdout to a " + "file." ) - raise BadArgsError() elif not is_file_writable(options.output_file): - log.error( - "Output file location (%s) is not a writable file.", options.output_file + raise OutputFileAccessError( + f"Output file location ({options.output_file}) is not a writable file." ) - raise OutputFileAccessError() def report_output_file_size(options, input_file, output_file): @@ -429,12 +408,8 @@ def report_output_file_size(options, input_file, output_file): explanation = "No reason for this increase is known. Please report this issue." log.warning( - textwrap.dedent( - f"""\ - The output file size is {ratio:.2f}× larger than the input file. - {explanation} - """ - ) + f"The output file size is {ratio:.2f}× larger than the input file.\n" + f"{explanation}" ) @@ -452,12 +427,11 @@ def check_dependency_versions(options): need_version='9.15', # limited by Travis CI / Ubuntu 14.04 backports ) if ghostscript.version() == '9.24': - log.error( + raise MissingDependencyError( "Ghostscript 9.24 contains serious regressions and is not " "supported. Please upgrade to Ghostscript 9.25 or use an older " "version." ) - return ExitCode.missing_dependency check_external_program( program='qpdf', package='qpdf', diff --git a/src/ocrmypdf/api.py b/src/ocrmypdf/api.py index acf60760..dc5da476 100644 --- a/src/ocrmypdf/api.py +++ b/src/ocrmypdf/api.py @@ -24,6 +24,7 @@ from tqdm import tqdm from .cli import parser from ._sync import run_pipeline +from ._validation import check_options class TqdmConsole: @@ -178,8 +179,5 @@ def ocrmypdf( # pylint: disable=unused-argument keep_temporary_files=None, ): options = create_options(**locals()) - return run(options) - - -def run(options): + check_options(options) return run_pipeline(options) diff --git a/tests/test_unpaper.py b/tests/test_unpaper.py index 8df1d5a2..887184cb 100644 --- a/tests/test_unpaper.py +++ b/tests/test_unpaper.py @@ -19,9 +19,10 @@ from os import fspath from unittest.mock import patch import pytest -from ocrmypdf.__main__ import parser + +from ocrmypdf.cli import parser from ocrmypdf._validation import check_options -from ocrmypdf.exceptions import ExitCode +from ocrmypdf.exceptions import ExitCode, MissingDependencyError from ocrmypdf.exec import unpaper # pytest.helpers is dynamic @@ -54,7 +55,7 @@ def test_no_unpaper(resources, no_outpdf): with patch("ocrmypdf.exec.unpaper.version") as mock_unpaper_version: mock_unpaper_version.side_effect = FileNotFoundError("unpaper") - with pytest.raises(SystemExit): + with pytest.raises(MissingDependencyError): check_options(options)