Abosrbing review changes. Linting issues to be resolved in followup commits.

Author: Bryan Sieber

src/core/actions.py

@@ -5,23 +5,30 @@
 # update this dict with additional action contexts
 
 
+class ActionError(Exception):
+    pass
+
+
 def get_action_context_by_key(key):
     if key not in module_dict:
-        assert False, "Unknown key requested"
+        raise ValueError("Unknown key requested")
     requested_module = module_dict.get(key)
     return get_action_context_from_module(action_module=requested_module)
 
 
 def get_action_context_from_module(action_module):
     context_map = {}
     for _, module in getmembers(action_module, ismodule):
-        # print(module)
         methods = getmembers(module, isfunction)
         for method_name, method in methods:
-            assert (
+            method_name_available = (
                 method_name
                 not in context_map.keys()  # pylint: disable=consider-iterating-dictionary
-            ), f"ACTION ERROR: Action with name  `{method_name}` already exists."
+            )
+            if not method_name_available:
+                raise ActionError(
+                    f"ACTION ERROR: Action with name  `{method_name}` already exists."
+                )
             context_map[method_name] = method
 
     return context_map

src/core/configurator.py

@@ -1,14 +1,21 @@
-import glob
 import json
-import os
-from typing import Dict, List, Tuple
+from pathlib import Path
+from typing import Any, Dict, List, Optional, Tuple
 
 from src.core import actions
 
 
+class ConfigError(Exception):
+    pass
+
+
+class ProcessError(Exception):
+    pass
+
+
 def per_file_process(
-    filename: str, ret_dict: dict, required_keys, action_key, filename_key
-) -> Tuple:
+    filename: str, ret_dict: Dict, required_keys, action_key, filename_key
+) -> Tuple[Optional[str], Optional[Dict[str, Any]]]:
     with open(filename, encoding="utf-8") as file:
         data = json.load(file)
         ret_dict.update(data)
@@ -19,19 +26,22 @@ def per_file_process(
         action = ret_dict.get("action")
         known_actions = actions.get_action_context_by_key(key=action_key)
         if action not in known_actions:
-            assert False, f"CONFIG ERROR: Unknown action `{action}`."
+            raise ConfigError(f"CONFIG ERROR: Unknown action `{action}`.")
 
         for key_str in required_keys:
             key_value = ret_dict.get(key_str)
-            assert (
-                key_value is not None
-            ), f"CONFIG ERROR: Required field `{key_str}` not found in {filename}."
+            if not key_value:
+                raise ConfigError(
+                    f"CONFIG ERROR: Required field `{key_str}` not found in {filename}."
+                )
 
         ret_key = ret_dict.get(filename_key)
-        assert ret_key is not None and ret_key in filename, (
-            f"CONFIG ERROR: Filename should contain value within key `{filename_key}`. The "
-            f"value {ret_key} from the key is expected to be in the filename. "
-        )
+        filename_valid = ret_key is not None and ret_key in filename
+        if not filename_valid:
+            raise ConfigError(
+                f"CONFIG ERROR: Filename should contain value within key `{filename_key}`. The "
+                f"value {ret_key} from the key is expected to be in the filename. "
+            )
 
         return ret_key, ret_dict
 
@@ -47,16 +57,17 @@ def process_all_files_in_path(
     if not config_map:
         config_map = {}
 
-    for filename in glob.glob(os.path.join(folder_path, "*.json")):
+    for filename in Path(folder_path).glob("*.json"):
         try:
-            if "TEMPLATE" in filename:
+            filename_s = str(filename)
+            if "TEMPLATE" in filename_s:
                 continue
-            key, value = process(filename)
+            key, value = process(filename_s)
             if key:
                 config_map[key] = value
-        except Exception as exception:  # pylint: disable=broad-except
+        except (ValueError, ConfigError, actions.ActionError) as exception:
             errors.append(exception)
 
     if errors:
-        assert False, f"PROCESS ERROR: errors exist: {errors}"
+        raise ProcessError(f"PROCESS ERROR: errors exist: {errors}")
     return config_map

src/jbi/whiteboard_actions/default.py

@@ -3,8 +3,8 @@
 
 
 def default_action(data, context):
-    print(services.bugzilla)
-    print(services.jira)
+    print(services.get_bugzilla())
+    print(services.get_jira())
 
 
 def default_helper():

tests/unit/core/test_configurator.py

@@ -1,7 +1,13 @@
 # pylint: disable=cannot-enumerate-pytest-fixtures
 import pytest
 
-from src.core.configurator import actions, per_file_process, process_all_files_in_path
+from src.core.configurator import (
+    ConfigError,
+    ProcessError,
+    actions,
+    per_file_process,
+    process_all_files_in_path,
+)
 from tests.unit.core import mock_actions
 
 
@@ -67,7 +73,7 @@ def test_file_processing_enabled_file_unknown_action():
     req_keys = ["enabled"]
     filename = "tests/unit/core/mock_config_files/unknown_action.json"
     actions.module_dict[action_key] = mock_actions
-    with pytest.raises(AssertionError):
+    with pytest.raises(ConfigError):
         per_file_process(
             filename=filename,
             required_keys=req_keys,
@@ -79,9 +85,9 @@ def test_file_processing_enabled_file_unknown_action():
 
 def test_config_path_throws_exception():
     def raise_except(filename):
-        raise Exception
+        raise ConfigError
 
-    with pytest.raises(AssertionError):
+    with pytest.raises(ProcessError):
         process_all_files_in_path(
             folder_path="tests/unit/core/mock_config_files/", process=raise_except
         )

tests/unit/jbi/test_jbi.py

@@ -6,7 +6,7 @@
 
 
 def test_mock_jbi_files():
-    with pytest.raises(AssertionError):
+    with pytest.raises(test_configurator.ProcessError):
         test_configurator.process_all_files_in_path(
             process=jbi_configuration.jbi_config_process,
             folder_path="tests/unit/jbi/mock_jbi_files",