From 93e19666072295bfd4bf861d004eac5c82454321 Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Thu, 3 Aug 2023 06:33:17 -0700 Subject: [PATCH] Feature catch template error (#1141) * Starting to implement catch_errors for sidecar_template * Fixed error printing * Fixed error printing * Added tests for catch_errors --- osxphotos/cli/export.py | 5 ++- osxphotos/cli/sidecar.py | 40 ++++++++++++++----- osxphotos/photoexporter.py | 6 ++- tests/custom_sidecar_bad.mako | 2 + tests/test_cli_export_sidecar_template.py | 48 +++++++++++++++++++++++ 5 files changed, 89 insertions(+), 12 deletions(-) create mode 100644 tests/custom_sidecar_bad.mako diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index 9cce4e6d..869b1b2f 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -355,6 +355,7 @@ from .verbose import get_verbose_console, verbose_print "strip_whitespace", "strip_lines", "skip_zero", + "catch_errors", "none", ] ), @@ -370,12 +371,14 @@ from .verbose import get_verbose_console, verbose_print "The `{filepath}` template variable may be used in the SIDECAR_FILENAME_TEMPLATE to refer to the filename of the " "photo being exported. " "OPTIONS is a comma-separated list of strings providing additional options to the template. " - "Valid options are: write_skipped, strip_whitespace, strip_lines, skip_zero, none. " + "Valid options are: write_skipped, strip_whitespace, strip_lines, skip_zero, catch_errors, none. " "write_skipped will cause the sidecar file to be written even if the photo is skipped during export. " "If write_skipped is not passed as an option, the sidecar file will not be written if the photo is skipped during export. " "strip_whitespace and strip_lines indicate whether or not to strip whitespace and blank lines, respectively, " "from the resulting sidecar file. " "skip_zero causes the sidecar file to be skipped if the rendered template is zero-length. " + "catch_errors causes errors in the template to be caught and logged but not raised. " + "Without catch_errors, osxphotos will abort the export if an error occurs in the template. " "For example, to create a sidecar file with extension .xmp using a template file named 'sidecar.mako' " "and write a sidecar for skipped photos and strip blank lines but not whitespace: " "`--sidecar-template sidecar.mako '{filepath}.xmp' write_skipped,strip_lines`. " diff --git a/osxphotos/cli/sidecar.py b/osxphotos/cli/sidecar.py index 5733b88d..57a01683 100644 --- a/osxphotos/cli/sidecar.py +++ b/osxphotos/cli/sidecar.py @@ -9,6 +9,7 @@ from typing import Callable import click from mako.template import Template +from osxphotos.cli.click_rich_echo import rich_echo_error from osxphotos.photoexporter import ExportResults from osxphotos.photoinfo import PhotoInfo from osxphotos.phototemplate import PhotoTemplate, RenderOptions @@ -54,6 +55,7 @@ def generate_user_sidecar( strip_lines = "strip_lines" in options write_skipped = "write_skipped" in options skip_zero = "skip_zero" in options + catch_errors = "catch_errors" in options if not write_skipped: # skip writing sidecar if photo not exported @@ -91,7 +93,7 @@ def generate_user_sidecar( ) verbose(f"Writing sidecar file [filepath]{template_filename}[/]") - _render_sidecar_and_write_data( + if error := _render_sidecar_and_write_data( template_file=template_file, photo=photo, template_filename=template_filename, @@ -99,11 +101,15 @@ def generate_user_sidecar( strip_whitespace=strip_whitespace, strip_lines=strip_lines, skip_zero=skip_zero, + catch_errors=catch_errors, verbose=verbose, dry_run=dry_run, - ) - sidecar_results.sidecar_user_written.append(template_filename) + ): + sidecar_results.sidecar_user_error.append((template_filename, error)) + else: + sidecar_results.sidecar_user_written.append(template_filename) + print(sidecar_results) return sidecar_results @@ -133,16 +139,28 @@ def _render_sidecar_and_write_data( strip_whitespace: bool, strip_lines: bool, skip_zero: bool, + catch_errors: bool, verbose: Callable[..., None], dry_run: bool, -): - """Render sidecar template and write data to file""" +) -> Exception | None: + """Render sidecar template and write data to file + + Returns: + None if no errors, otherwise Exception if catch_errors is True + If catch_errors is False, raises exception if error + """ sidecar = get_template(template_file) - sidecar_data = sidecar.render( - photo=photo, - sidecar_path=pathlib.Path(template_filename), - photo_path=pathlib.Path(filepath), - ) + try: + sidecar_data = sidecar.render( + photo=photo, + sidecar_path=pathlib.Path(template_filename), + photo_path=pathlib.Path(filepath), + ) + except Exception as e: + if catch_errors: + rich_echo_error(f"[error]Error rendering sidecar template: {e}[/]") + return e + raise e if strip_whitespace: # strip whitespace @@ -159,3 +177,5 @@ def _render_sidecar_and_write_data( return with open(template_filename, "w") as f: f.write(sidecar_data) + + return None diff --git a/osxphotos/photoexporter.py b/osxphotos/photoexporter.py index c821e806..a73f85b4 100644 --- a/osxphotos/photoexporter.py +++ b/osxphotos/photoexporter.py @@ -292,6 +292,7 @@ class ExportResults: "sidecar_xmp_written", "sidecar_user_written", "sidecar_user_skipped", + "sidecar_user_error", "skipped", "skipped_album", "to_touch", @@ -325,6 +326,7 @@ class ExportResults: sidecar_xmp_written=None, sidecar_user_written=None, sidecar_user_skipped=None, + sidecar_user_error=None, skipped=None, skipped_album=None, to_touch=None, @@ -372,6 +374,7 @@ class ExportResults: files += [x[0] for x in self.exiftool_warning] files += [x[0] for x in self.exiftool_error] files += [x[0] for x in self.error] + files += [x[0] for x in self.sidecar_user_error] return list(set(files)) @@ -1439,7 +1442,8 @@ class PhotoExporter: if options.export_as_hardlink: try: if aae_dest.exists() and any( - [options.overwrite, options.update, options.force_update]): + [options.overwrite, options.update, options.force_update] + ): try: options.fileutil.unlink(aae_dest) except Exception as e: diff --git a/tests/custom_sidecar_bad.mako b/tests/custom_sidecar_bad.mako new file mode 100644 index 00000000..19162b1a --- /dev/null +++ b/tests/custom_sidecar_bad.mako @@ -0,0 +1,2 @@ +<%doc> This will cause a syntax error. +${1/0} \ No newline at end of file diff --git a/tests/test_cli_export_sidecar_template.py b/tests/test_cli_export_sidecar_template.py index 140482c6..2234d04e 100644 --- a/tests/test_cli_export_sidecar_template.py +++ b/tests/test_cli_export_sidecar_template.py @@ -544,3 +544,51 @@ def test_export_sidecar_template_skip_zero(): sidecar_file = pathlib.Path(SIDECAR_FILENAME_NO_KEYWORD) assert not sidecar_file.exists() + + +def test_export_sidecar_template_error(): + """test basic export with --sidecar-template that generates an error""" + runner = CliRunner() + cwd = os.getcwd() + # pylint: disable=not-context-manager + with runner.isolated_filesystem(): + result = runner.invoke( + export, + [ + "--library", + os.path.join(cwd, PHOTOS_DB), + ".", + "-V", + "--uuid", + PHOTO_UUID, + "--sidecar-template", + os.path.join(cwd, "tests", "custom_sidecar_bad.mako"), + "{filepath}.txt", + "none", + ], + ) + assert result.exit_code != 0 + + +def test_export_sidecar_template_catch_error(): + """test basic export with --sidecar-template that catches an error""" + runner = CliRunner() + cwd = os.getcwd() + # pylint: disable=not-context-manager + with runner.isolated_filesystem(): + result = runner.invoke( + export, + [ + "--library", + os.path.join(cwd, PHOTOS_DB), + ".", + "-V", + "--uuid", + PHOTO_UUID, + "--sidecar-template", + os.path.join(cwd, "tests", "custom_sidecar_bad.mako"), + "{filepath}.txt", + "catch_errors", + ], + ) + assert result.exit_code == 0