diff --git a/speedwagon/config/workflow.py b/speedwagon/config/workflow.py index 294472bd3..bfb7e630d 100644 --- a/speedwagon/config/workflow.py +++ b/speedwagon/config/workflow.py @@ -5,15 +5,18 @@ import collections.abc import os import io +import logging +import pathlib from typing import ( - Optional, + Any, + Callable, Dict, + Iterator, List, - TYPE_CHECKING, - Callable, + Optional, + Protocol, TextIO, - Any, - Iterator, + TYPE_CHECKING, ) import yaml @@ -45,6 +48,8 @@ "AbsWorkflowBackend" ] +logger = logging.getLogger(__name__) + class AbsSettingsSerializer(abc.ABC): # pylint: disable=R0903 @abc.abstractmethod @@ -411,39 +416,83 @@ def get_workflow_config_from_yaml_file( raise speedwagon.exceptions.MissingConfiguration( f"Workflow '{workflow_name}' not found in YAML data." ) + return parse_workflow_yaml_config(yaml_data[workflow_name]) + except yaml.YAMLError as yaml_error: + raise speedwagon.exceptions.FileFormatError( + "Error parsing YAML file" + ) from yaml_error + + +def parse_workflow_yaml_config( + workflow_config: List[Dict[str, str]] +) -> SettingsData: + try: + for section in workflow_config: + if any((("name" not in section), ("value" not in section))): + raise speedwagon.exceptions.FileFormatError( + f"{section} missing 'name' or 'value'" + ) + except ( + TypeError, + speedwagon.exceptions.FileFormatError, + ) as format_error: + raise speedwagon.exceptions.FileFormatError( + "Config file format not valid" + ) from format_error + + return {entry["name"]: entry["value"] for entry in workflow_config} + + +def get_workflow_config_from_yaml_ignore_incomplete( + fp: TextIO, workflow_name: str +) -> SettingsData: + start_point = fp.tell() + fp.seek(0) + res = fp.read() + if res.strip() == "": + return {} + fp.seek(start_point) + try: + yaml_data = yaml.safe_load(fp) + if workflow_name not in yaml_data: + logger.debug( + "Workflow '%s' not found in YAML data.", workflow_name + ) + return {} workflow_config = yaml_data[workflow_name] - try: - for section in workflow_config: - if any( - ( - ("name" not in section), - ("value" not in section) - ) - ): - raise speedwagon.exceptions.FileFormatError( - f"{section} missing 'name' or 'value'" - ) - except ( - TypeError, - speedwagon.exceptions.FileFormatError, - ) as format_error: - raise speedwagon.exceptions.FileFormatError( - "Config file format not valid" - ) from format_error + if not workflow_config: + return {} + return parse_workflow_yaml_config(workflow_config) - return { - entry['name']: entry['value'] - for entry in workflow_config - } except yaml.YAMLError as yaml_error: raise speedwagon.exceptions.FileFormatError( "Error parsing YAML file" ) from yaml_error -def get_workflow_options(yml_file: str, workflow_name: str) -> SettingsData: - with open(yml_file, "r", encoding="utf-8") as fp: - return speedwagon.config.workflow.get_workflow_config_from_yaml_file( - fp, - workflow_name - ) +# pylint: disable-next=too-few-public-methods +class GetWorkflowOptionsFromYamlProtocol(Protocol): + """Protocol for getting workflow options from YAML file.""" + def __call__(self, fp: TextIO, workflow_name: str) -> SettingsData: + ... + + +def get_workflow_options( + yml_file: str, + workflow_name: str, + strategy: GetWorkflowOptionsFromYamlProtocol = ( + get_workflow_config_from_yaml_file + ), + allow_missing: bool = False, +) -> SettingsData: + try: + with pathlib.Path(yml_file).open("r", encoding="utf-8") as fp: + return strategy( + fp, + workflow_name + ) + except FileNotFoundError: + if allow_missing: + logger.warning('Expected config file missing: %s', yml_file) + return {} + raise diff --git a/speedwagon/frontend/qtwidgets/gui_startup.py b/speedwagon/frontend/qtwidgets/gui_startup.py index f00cbbe69..5d9a79a50 100644 --- a/speedwagon/frontend/qtwidgets/gui_startup.py +++ b/speedwagon/frontend/qtwidgets/gui_startup.py @@ -50,7 +50,7 @@ ) from speedwagon.config.common import DEFAULT_CONFIG_DIRECTORY_NAME from speedwagon.config import plugins as plugin_config -from speedwagon.config.workflow import WORKFLOWS_SETTINGS_YML_FILE_NAME +from speedwagon.config import workflow as workflow_config from speedwagon.utils import ( get_desktop_path, validate_user_input, @@ -91,6 +91,8 @@ T = TypeVar("T") +module_logger = logging.getLogger(__name__) + system_info_report_formatters: DefaultDict[ str, Callable[[info.SystemInfo], str] ] = collections.defaultdict( @@ -686,6 +688,27 @@ def import_workflow_config( parent.logger.error("Failed to load workflow. Reason: %s", error) +def get_workflow_options_if_config_file_exists( + config_file: str, + workflow_name: str +) -> SettingsData: + + if not os.path.exists(config_file): + module_logger.debug( + "Unable to locate workflow settings. " + 'Reason: "%s" file does not exist.', + config_file, + ) + return {} + return speedwagon.config.workflow.get_workflow_options( + config_file, + workflow_name, + strategy=( + workflow_config.get_workflow_config_from_yaml_ignore_incomplete + ) + ) + + class StartQtThreaded(GuiStarter): """Start a Qt Widgets base app using threads for job workers.""" @@ -766,16 +789,17 @@ def set_workflow_config_backend_factory( source_application_settings=settings_resolver, workflow_backend=factory, ) + config_file = os.path.join( + self.config_files_locator.get_app_data_dir(), + workflow_config.WORKFLOWS_SETTINGS_YML_FILE_NAME, + ) - self.get_workflow_options_strategy = lambda workflow_name: ( - speedwagon.config.workflow.get_workflow_options( - os.path.join( - self.config_files_locator.get_app_data_dir(), - WORKFLOWS_SETTINGS_YML_FILE_NAME, - ), - workflow_name, + self.get_workflow_options_strategy =\ + lambda workflow: get_workflow_options_if_config_file_exists( + config_file, + workflow ) - ) + self.get_plugin_data_strategy = lambda: ( runner_strategies.get_plugin_data( self.config_files_locator.get_config_file() @@ -1244,7 +1268,7 @@ def default_get_workflow_options_strategy( return speedwagon.config.workflow.get_workflow_options( os.path.join( config_files_locator.get_app_data_dir(), - speedwagon.config.workflow.WORKFLOWS_SETTINGS_YML_FILE_NAME, + workflow_config.WORKFLOWS_SETTINGS_YML_FILE_NAME, ), workflow_name, ) @@ -1344,7 +1368,7 @@ def __init__( ] = None self.options: typing.Optional[SettingsData] = None self.workflow: typing.Optional[AbsWorkflow] = None - self.logger = logger or logging.getLogger(__name__) + self.logger = logger or logging.getLogger() def load_json_string(self, data: str) -> None: """Load json data containing options and workflow info. @@ -1446,7 +1470,7 @@ def _run_workflow( dialog_box ) ) - dialog_box.attach_logger(job_manager.logger) + dialog_box.attach_logger(self.logger) job_manager.workflow_loader_strategy = self.load_workflow_strategy @@ -1627,7 +1651,8 @@ def build_request_settings_dialog( ) -> QtWidgets.QDialog: settings_builder = LocalSettingsBuilder() workflow_settings_yaml = os.path.join( - settings_locator.get_app_data_dir(), WORKFLOWS_SETTINGS_YML_FILE_NAME + settings_locator.get_app_data_dir(), + workflow_config.WORKFLOWS_SETTINGS_YML_FILE_NAME ) settings_builder.on_open_config_dir = functools.partial( dialog.settings.open_settings_dir, diff --git a/speedwagon/runner.py b/speedwagon/runner.py index 9546b9396..b6e36f5b6 100644 --- a/speedwagon/runner.py +++ b/speedwagon/runner.py @@ -977,7 +977,8 @@ def run( [ WorkerLogHandler( lambda record: callbacks.log( - text=record.message, level=record.levelno + text=record.getMessage(), + level=record.levelno ) ) ], diff --git a/speedwagon/startup.py b/speedwagon/startup.py index d6f3c89d3..f0b717c30 100644 --- a/speedwagon/startup.py +++ b/speedwagon/startup.py @@ -43,10 +43,8 @@ import speedwagon.job import speedwagon.config import speedwagon.info -from speedwagon.config.workflow import ( - default_backend_factory, - AbsWorkflowBackend, -) +from speedwagon.config import workflow as workflow_config + from speedwagon.config.plugins import ( get_whitelisted_plugins_from_config_data, read_settings_data_plugins @@ -232,7 +230,7 @@ def __init__(self, strategy: Optional[AbsStarter] = None) -> None: ) config_backend_factory = functools.partial( - speedwagon.config.workflow.default_backend_factory, + workflow_config.default_backend_factory, config_directory_name=self.application_config_directory_name, ) self.strategy = strategy or StartQtThreaded( @@ -256,7 +254,7 @@ def run(self, app=None) -> int: """Run Speedwagon.""" self.strategy.set_application_name(self.application_name) config_backend = functools.partial( - default_backend_factory, + workflow_config.default_backend_factory, config_directory_name=self.application_config_directory_name, ) @@ -397,14 +395,18 @@ def json_startup( try: default_yaml_file_name =\ speedwagon.config.workflow.WORKFLOWS_SETTINGS_YML_FILE_NAME + config_loader_strategy =\ + workflow_config.get_workflow_config_from_yaml_ignore_incomplete startup_strategy.get_workflow_options_strategy = ( lambda workflow_name: ( - speedwagon.config.workflow.get_workflow_options( + workflow_config.get_workflow_options( os.path.join( config_file_locator.get_app_data_dir(), default_yaml_file_name, ), workflow_name, + allow_missing=True, + strategy=config_loader_strategy, ) ) ) @@ -550,7 +552,10 @@ def set_application_name(self, name: str) -> None: # noqa: B027 """ def set_workflow_config_backend_factory( # noqa: B027 - self, factory: Callable[[speedwagon.job.Workflow], AbsWorkflowBackend] + self, + factory: Callable[ + [speedwagon.job.Workflow], workflow_config.AbsWorkflowBackend + ] ) -> None: """Set the workflow config backend factory. @@ -588,10 +593,10 @@ def default_get_workflow_options_strategy( config_files_locator = config_files_locator or StandardConfigFileLocator( config_directory_prefix=DEFAULT_CONFIG_DIRECTORY_NAME ) - return speedwagon.config.workflow.get_workflow_options( + return workflow_config.get_workflow_options( os.path.join( config_files_locator.get_app_data_dir(), - speedwagon.config.workflow.WORKFLOWS_SETTINGS_YML_FILE_NAME, + workflow_config.WORKFLOWS_SETTINGS_YML_FILE_NAME, ), workflow_name, ) diff --git a/tests/test_config.py b/tests/test_config.py index b83b429a1..c22bdffb9 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -4,7 +4,7 @@ import io import logging from typing import Optional, List, TYPE_CHECKING, Any, Dict -from unittest.mock import Mock, patch, mock_open, ANY, call +from unittest.mock import Mock, patch, mock_open, ANY, MagicMock, create_autospec import pytest @@ -939,6 +939,82 @@ def test_get_workflow_config_from_yaml_file_wrong_format_data(bad_yml_data): file_handler, workflow_name="Generate MARC.XML Files" ) +@pytest.mark.parametrize( + "yaml_data,workflow_name,expected", + [ + ( + """ +Generate MARC.XML Files: + - name: Getmarc server url + value: dummy +Generate OCR Files: + - name: Tesseract data file location + value: /Users/testuser/tesseract_data + """.lstrip(), + "Generate MARC.XML Files", + { + "Getmarc server url": "dummy", + } + ), + ( + "", + "Generate MARC.XML Files", + {} + ), + ( + """ +Generate MARC.XML Files: + - name: Getmarc server url + value: dummy +Generate OCR Files: + - name: Tesseract data file location + value: /Users/testuser/tesseract_data + """.lstrip(), + "some workflow without configs", + {} + ), + ( + """ +Generate MARC.XML Files: +Generate OCR Files: + """.lstrip(), + "Generate MARC.XML Files", + {} + ), + ] +) +def test_get_workflow_config_from_yaml_file_ignore_incomplete(yaml_data, workflow_name,expected): + file_handler = io.StringIO(yaml_data) + assert speedwagon.config.workflow.get_workflow_config_from_yaml_ignore_incomplete( + file_handler, workflow_name=workflow_name + ) == expected + +@pytest.mark.parametrize( + "yaml_data", + [ + """ + Generate MARC.XML Files: + - key: Tesseract data file location + value: /Users/testuser/tesseract_data + """.lstrip(), + """ + app: + name: MyCoolApp + debug: true + invalid-indentation: + - item1 + : item2 # Incorrect use of colon + """.lstrip(), + ] +) +def test_get_workflow_config_from_yaml_file_ignore_incomplete_invalid_data_raises(yaml_data): + + file_handler = io.StringIO(yaml_data) + with pytest.raises(speedwagon.exceptions.FileFormatError): + speedwagon.config.workflow.get_workflow_config_from_yaml_ignore_incomplete( + file_handler, workflow_name="Generate MARC.XML Files" + ) + def test_parse_plugin_data(): settings = { 'global': {}, @@ -955,3 +1031,35 @@ def test_parse_plugin_data_warns_on_invalid_data(caplog): results = speedwagon.config.plugins.parse_plugin_data(settings) assert caplog.messages assert results['bacon'] == {'bacon': True} + +def test_get_workflow_options(monkeypatch): + yaml_file = "a_real_file.yml" + monkeypatch.setattr(speedwagon.config.workflow.pathlib, "Path", MagicMock()) + strategy = create_autospec(speedwagon.config.workflow.GetWorkflowOptionsFromYamlProtocol, instance=True) + speedwagon.config.workflow.get_workflow_options( + yaml_file, + "some workflow", + strategy=strategy + ) + strategy.assert_called_once() + +def test_get_workflow_options_missing_file_without_missing_file_raises(): + yaml_file = "missing_file.yml" + strategy = create_autospec(speedwagon.config.workflow.GetWorkflowOptionsFromYamlProtocol, instance=True) + with pytest.raises(FileNotFoundError): + speedwagon.config.workflow.get_workflow_options( + yaml_file, + "some workflow", + strategy=strategy, + allow_missing=False + ) + +def test_get_workflow_options_missing_file_with_missing_file_get_empty_dict(): + yaml_file = "missing_file.yml" + strategy = create_autospec(speedwagon.config.workflow.GetWorkflowOptionsFromYamlProtocol, instance=True) + assert speedwagon.config.workflow.get_workflow_options( + yaml_file, + "some workflow", + strategy=strategy, + allow_missing=True + ) == {}