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

@@ -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())

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)

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',

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)

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)