From 1d2ccf566845d5dab5280fb3dfcc738059f9ece2 Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Thu, 3 Aug 2023 07:03:38 -0700 Subject: [PATCH 1/4] Added --post-command-break/catch --- osxphotos/cli/export.py | 70 +++++++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index 869b1b2ff..3ab6bd909 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -1,5 +1,7 @@ """export command for osxphotos CLI""" +from __future__ import annotations + import atexit import inspect import os @@ -9,7 +11,7 @@ import subprocess import sys import time -from typing import Iterable, List, Optional, Tuple +from typing import Any, Callable, Iterable, List, Optional, Tuple import click @@ -647,7 +649,7 @@ "If present, this file will be read after the export is completed and any rules found in the file " "will be added to the list of rules to keep. " "This file uses the same format as a .gitignore file and should contain one rule per line; " - "lines starting with a `#` will be ignored. " + "lines starting with a `#` will be ignored. ", ) @click.option( "--add-exported-to-album", @@ -688,6 +690,22 @@ [click.Choice(POST_COMMAND_CATEGORIES, case_sensitive=False), TemplateString()] ), ) +@click.option( + "--post-command-break", + is_flag=True, + help="If specified and a --post-command fails, catch and log the error then stop running any other --post-command commands for the current photo; " + "--post-command will continue to run for the next photo. " + "The default behavior or --post-command is abort the export if a command encounters an error. " + "See also, --post-command-catch.", +) +@click.option( + "--post-command-catch", + is_flag=True, + help="If specified, catch and log any errors when running --post-command but continue " + "running any other --post-command commands for the current photo. " + "The default behavior or --post-command is abort the export if a command encounters an error. " + "See also --post-command-break.", +) @click.option( "--post-function", metavar="filename.py::function", @@ -910,6 +928,8 @@ def export( place, portrait, post_command, + post_command_break, + post_command_catch, post_function, preview, preview_if_missing, @@ -1138,6 +1158,8 @@ def export( place = cfg.place portrait = cfg.portrait post_command = cfg.post_command + post_command_break = cfg.post_command_break + post_command_catch = cfg.post_command_catch post_function = cfg.post_function preview = cfg.preview preview_if_missing = cfg.preview_if_missing @@ -1575,7 +1597,8 @@ def cleanup_lock_files(): export_dir=dest, dry_run=dry_run, exiftool_path=exiftool_path, - export_db=export_db, + break_on_error=post_command_break, + catch_errors=post_command_catch, verbose=verbose, ) @@ -2590,7 +2613,7 @@ def collect_files_to_keep( KEEP_RULEs = [] # parse .osxphotos_keep file if it exists - keep_file : pathlib.Path = export_dir / ".osxphotos_keep" + keep_file: pathlib.Path = export_dir / ".osxphotos_keep" if keep_file.is_file(): for line in keep_file.read_text().splitlines(): line = line.rstrip("\r\n") @@ -2604,10 +2627,10 @@ def collect_files_to_keep( KEEP_RULEs.append(k.replace(export_dir_str, "")) else: KEEP_RULEs.append(k) - + if not KEEP_RULEs: return [], [] - + # have some rules to apply matcher = osxphotos.gitignorefile.parse_pattern_list(KEEP_RULEs, export_dir) keepers = [] @@ -2841,16 +2864,19 @@ def write_extended_attributes( def run_post_command( - photo, - post_command, - export_results, - export_dir, - dry_run, - exiftool_path, - export_db, - verbose, + photo: osxphotos.PhotoInfo, + post_command: tuple[tuple[str, str]], + export_results: ExportResults, + export_dir: str | pathlib.Path, + dry_run: bool, + exiftool_path: str, + break_on_error: bool, + catch_errors: bool, + verbose: Callable[[Any], None], ): + """Run --post-command commands""" # todo: pass in RenderOptions from export? (e.g. so it contains strip, etc?) + for category, command_template in post_command: files = getattr(export_results, category) for f in files: @@ -2864,7 +2890,6 @@ def run_post_command( if command: verbose(f'Running command: "{command}"') if not dry_run: - args = shlex.split(command) cwd = pathlib.Path(f).parent run_error = None run_results = None @@ -2873,11 +2898,16 @@ def run_post_command( except Exception as e: run_error = e finally: - run_error = run_error or run_results.returncode - if run_error: - rich_echo_error( - f'[error]Error running command "{command}": {run_error}' - ) + returncode = run_results.returncode if run_results else None + if run_error or returncode: + error_str = f'Error running command "{command}": return code: {returncode}, exception: {run_error}' + if break_on_error or catch_errors: + rich_echo_error(f"[error]{error_str}[/]") + else: + raise RuntimeError(error_str) + if break_on_error: + # break out of loop and return + return def render_and_validate_report(report: str, exiftool_path: str, export_dir: str) -> str: From 7ad77939c5b9ed4168182d9c351dc75a32c60aea Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Thu, 3 Aug 2023 07:03:38 -0700 Subject: [PATCH 2/4] Added --post-command-break/catch --- osxphotos/cli/export.py | 70 +++++++++++++++++++++++++++++------------ 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index 869b1b2ff..3ab6bd909 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -1,5 +1,7 @@ """export command for osxphotos CLI""" +from __future__ import annotations + import atexit import inspect import os @@ -9,7 +11,7 @@ import subprocess import sys import time -from typing import Iterable, List, Optional, Tuple +from typing import Any, Callable, Iterable, List, Optional, Tuple import click @@ -647,7 +649,7 @@ "If present, this file will be read after the export is completed and any rules found in the file " "will be added to the list of rules to keep. " "This file uses the same format as a .gitignore file and should contain one rule per line; " - "lines starting with a `#` will be ignored. " + "lines starting with a `#` will be ignored. ", ) @click.option( "--add-exported-to-album", @@ -688,6 +690,22 @@ [click.Choice(POST_COMMAND_CATEGORIES, case_sensitive=False), TemplateString()] ), ) +@click.option( + "--post-command-break", + is_flag=True, + help="If specified and a --post-command fails, catch and log the error then stop running any other --post-command commands for the current photo; " + "--post-command will continue to run for the next photo. " + "The default behavior or --post-command is abort the export if a command encounters an error. " + "See also, --post-command-catch.", +) +@click.option( + "--post-command-catch", + is_flag=True, + help="If specified, catch and log any errors when running --post-command but continue " + "running any other --post-command commands for the current photo. " + "The default behavior or --post-command is abort the export if a command encounters an error. " + "See also --post-command-break.", +) @click.option( "--post-function", metavar="filename.py::function", @@ -910,6 +928,8 @@ def export( place, portrait, post_command, + post_command_break, + post_command_catch, post_function, preview, preview_if_missing, @@ -1138,6 +1158,8 @@ def export( place = cfg.place portrait = cfg.portrait post_command = cfg.post_command + post_command_break = cfg.post_command_break + post_command_catch = cfg.post_command_catch post_function = cfg.post_function preview = cfg.preview preview_if_missing = cfg.preview_if_missing @@ -1575,7 +1597,8 @@ def cleanup_lock_files(): export_dir=dest, dry_run=dry_run, exiftool_path=exiftool_path, - export_db=export_db, + break_on_error=post_command_break, + catch_errors=post_command_catch, verbose=verbose, ) @@ -2590,7 +2613,7 @@ def collect_files_to_keep( KEEP_RULEs = [] # parse .osxphotos_keep file if it exists - keep_file : pathlib.Path = export_dir / ".osxphotos_keep" + keep_file: pathlib.Path = export_dir / ".osxphotos_keep" if keep_file.is_file(): for line in keep_file.read_text().splitlines(): line = line.rstrip("\r\n") @@ -2604,10 +2627,10 @@ def collect_files_to_keep( KEEP_RULEs.append(k.replace(export_dir_str, "")) else: KEEP_RULEs.append(k) - + if not KEEP_RULEs: return [], [] - + # have some rules to apply matcher = osxphotos.gitignorefile.parse_pattern_list(KEEP_RULEs, export_dir) keepers = [] @@ -2841,16 +2864,19 @@ def write_extended_attributes( def run_post_command( - photo, - post_command, - export_results, - export_dir, - dry_run, - exiftool_path, - export_db, - verbose, + photo: osxphotos.PhotoInfo, + post_command: tuple[tuple[str, str]], + export_results: ExportResults, + export_dir: str | pathlib.Path, + dry_run: bool, + exiftool_path: str, + break_on_error: bool, + catch_errors: bool, + verbose: Callable[[Any], None], ): + """Run --post-command commands""" # todo: pass in RenderOptions from export? (e.g. so it contains strip, etc?) + for category, command_template in post_command: files = getattr(export_results, category) for f in files: @@ -2864,7 +2890,6 @@ def run_post_command( if command: verbose(f'Running command: "{command}"') if not dry_run: - args = shlex.split(command) cwd = pathlib.Path(f).parent run_error = None run_results = None @@ -2873,11 +2898,16 @@ def run_post_command( except Exception as e: run_error = e finally: - run_error = run_error or run_results.returncode - if run_error: - rich_echo_error( - f'[error]Error running command "{command}": {run_error}' - ) + returncode = run_results.returncode if run_results else None + if run_error or returncode: + error_str = f'Error running command "{command}": return code: {returncode}, exception: {run_error}' + if break_on_error or catch_errors: + rich_echo_error(f"[error]{error_str}[/]") + else: + raise RuntimeError(error_str) + if break_on_error: + # break out of loop and return + return def render_and_validate_report(report: str, exiftool_path: str, export_dir: str) -> str: From 126d0e28d0a80e0724c53a9857a901d787da467a Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Sat, 5 Aug 2023 07:58:20 -0700 Subject: [PATCH 3/4] Added --post-command-error and tests --- osxphotos/cli/export.py | 46 ++++++++++++++------------------- tests/test_cli.py | 57 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 29 deletions(-) diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index 3ab6bd909..7bcc12093 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -11,7 +11,7 @@ import subprocess import sys import time -from typing import Any, Callable, Iterable, List, Optional, Tuple +from typing import Any, Callable, Iterable, List, Literal, Optional, Tuple import click @@ -685,26 +685,20 @@ "COMMAND is an osxphotos template string, for example: '--post-command exported \"echo {filepath|shell_quote} >> {export_dir}/exported.txt\"', " "which appends the full path of all exported files to the file 'exported.txt'. " "You can run more than one command by repeating the '--post-command' option with different arguments. " + "See also --post-command-error and --post-function." "See Post Command below.", type=click.Tuple( [click.Choice(POST_COMMAND_CATEGORIES, case_sensitive=False), TemplateString()] ), ) @click.option( - "--post-command-break", - is_flag=True, - help="If specified and a --post-command fails, catch and log the error then stop running any other --post-command commands for the current photo; " - "--post-command will continue to run for the next photo. " - "The default behavior or --post-command is abort the export if a command encounters an error. " - "See also, --post-command-catch.", -) -@click.option( - "--post-command-catch", - is_flag=True, - help="If specified, catch and log any errors when running --post-command but continue " - "running any other --post-command commands for the current photo. " - "The default behavior or --post-command is abort the export if a command encounters an error. " - "See also --post-command-break.", + "--post-command-error", + help="Specify either `continue` or `break` to control behavior when a post-command fails. " + "If `continue`, osxphotos will log the error and continue processing. " + "If `break`, osxphotos will stop processing any additional --post-command commands for the current photo " + "but will continue with the export. " + "Without --post-command-error, osxphotos will abort the export if a post-command encounters an error. ", + type=click.Choice(["continue", "break"], case_sensitive=False), ) @click.option( "--post-function", @@ -928,8 +922,7 @@ def export( place, portrait, post_command, - post_command_break, - post_command_catch, + post_command_error, post_function, preview, preview_if_missing, @@ -1158,8 +1151,7 @@ def export( place = cfg.place portrait = cfg.portrait post_command = cfg.post_command - post_command_break = cfg.post_command_break - post_command_catch = cfg.post_command_catch + post_command_error = cfg.post_command_error post_function = cfg.post_function preview = cfg.preview preview_if_missing = cfg.preview_if_missing @@ -1597,8 +1589,7 @@ def cleanup_lock_files(): export_dir=dest, dry_run=dry_run, exiftool_path=exiftool_path, - break_on_error=post_command_break, - catch_errors=post_command_catch, + on_error=post_command_error, verbose=verbose, ) @@ -2870,8 +2861,7 @@ def run_post_command( export_dir: str | pathlib.Path, dry_run: bool, exiftool_path: str, - break_on_error: bool, - catch_errors: bool, + on_error: Literal["break", "continue"] | None, verbose: Callable[[Any], None], ): """Run --post-command commands""" @@ -2900,14 +2890,16 @@ def run_post_command( finally: returncode = run_results.returncode if run_results else None if run_error or returncode: + # there was an error running the command error_str = f'Error running command "{command}": return code: {returncode}, exception: {run_error}' - if break_on_error or catch_errors: - rich_echo_error(f"[error]{error_str}[/]") - else: + rich_echo_error(f"[error]{error_str}[/]") + if not on_error: + # no error handling specified, raise exception raise RuntimeError(error_str) - if break_on_error: + if on_error == "break": # break out of loop and return return + # else on_error must be continue def render_and_validate_report(report: str, exiftool_path: str, export_dir: str) -> str: diff --git a/tests/test_cli.py b/tests/test_cli.py index e8956f0dc..99c909dd9 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -8644,14 +8644,67 @@ def test_export_post_command_bad_command(): ".", "--post-command", "exported", - "foobar {filepath.name|shell_quote} >> {export_dir}/exported.txt", + "false", "--name", "Park", "--skip-original-if-edited", ], ) + assert result.exit_code != 0 + + +def test_export_post_command_bad_command_continue(): + """Test --post-command with bad command with --post-command-error=continue""" + + runner = CliRunner() + cwd = os.getcwd() + # pylint: disable=not-context-manager + with runner.isolated_filesystem(): + result = runner.invoke( + cli_main, + [ + "export", + "--db", + os.path.join(cwd, PHOTOS_DB_15_7), + ".", + "--post-command", + "exported", + "false", + "--post-command-error", + "continue", + "--name", + "wedding", + ], + ) + assert result.exit_code == 0 + assert result.output.count("Error running command") == 2 + + +def test_export_post_command_bad_command_break(): + """Test --post-command with bad command with --post-command-error=break""" + + runner = CliRunner() + cwd = os.getcwd() + # pylint: disable=not-context-manager + with runner.isolated_filesystem(): + result = runner.invoke( + cli_main, + [ + "export", + "--db", + os.path.join(cwd, PHOTOS_DB_15_7), + ".", + "--post-command", + "exported", + "false", + "--post-command-error", + "break", + "--name", + "wedding", + ], + ) assert result.exit_code == 0 - assert 'Error running command "foobar' in result.output + assert result.output.count("Error running command") == 1 def test_export_post_command_bad_option_1(): From 4676a51ba9a007bf06a6e108042e82f4d3f49f47 Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Sat, 5 Aug 2023 08:11:17 -0700 Subject: [PATCH 4/4] Fixed help text for --post-command-error --- osxphotos/cli/export.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osxphotos/cli/export.py b/osxphotos/cli/export.py index 7bcc12093..af17adaec 100644 --- a/osxphotos/cli/export.py +++ b/osxphotos/cli/export.py @@ -693,7 +693,8 @@ ) @click.option( "--post-command-error", - help="Specify either `continue` or `break` to control behavior when a post-command fails. " + metavar="ACTION", + help="Specify either `continue` or `break` for ACTION to control behavior when a post-command fails. " "If `continue`, osxphotos will log the error and continue processing. " "If `break`, osxphotos will stop processing any additional --post-command commands for the current photo " "but will continue with the export. "