Refactor logging structure

src/aind_behavior_services/launcher/__init__.py

@@ -24,6 +24,7 @@
 from aind_behavior_services.launcher.services import Services
 from aind_behavior_services.utils import model_from_json_file, utcnow
 
+
 TRig = TypeVar("TRig", bound=AindBehaviorRigModel)  # pylint: disable=invalid-name
 TSession = TypeVar("TSession", bound=AindBehaviorSessionModel)  # pylint: disable=invalid-name
 TTaskLogic = TypeVar("TTaskLogic", bound=AindBehaviorTaskLogicModel)  # pylint: disable=invalid-name
@@ -60,9 +61,11 @@ def __init__(
         self.temp_dir = self.abspath(temp_dir) / secrets.token_hex(nbytes=16)
         self.temp_dir.mkdir(parents=True, exist_ok=True)
         self.logger = (
-            logger if logger is not None else logging_helper.default_logger_factory(self.temp_dir / "launcher.log")
+            logger
+            if logger is not None
+            else logging_helper.default_logger_builder(logging.getLogger(__name__), self.temp_dir / "launcher.log")
         )
-        self._ui_helper = ui_helper.UIHelper(logger=self.logger)
+        self._ui_helper = ui_helper.UIHelper()
         self._services = services
         self._cli_args = self._cli_wrapper()
 
@@ -132,8 +135,6 @@ def _post_init_(self, validate: bool = True) -> None:
             self.logger.setLevel(logging.DEBUG)
         if cli_args.create_directories is True:
             self._create_directory_structure()
-        if self._services is not None:
-            self.services.register_logger(self.logger)
         if validate:
             self.validate()
 
@@ -610,3 +611,4 @@ def _cli_wrapper(cls) -> argparse.Namespace:
     def _copy_tmp_directory(self, dst: os.PathLike) -> None:
         dst = Path(dst) / ".launcher"
         shutil.copytree(self.temp_dir, dst, dirs_exist_ok=True)
+

src/aind_behavior_services/launcher/_service.py

@@ -1,16 +1,9 @@
-import logging
-from typing import Optional, Protocol
+from typing import Protocol
 
 
 class IService(Protocol):
     """A minimal interface to ensure that services have expected functionality."""
 
-    def __init__(self, *args, logger: Optional[logging.Logger] = None, **kwargs) -> None: ...
+    def __init__(self, *args, **kwargs) -> None: ...
 
     def validate(self, *args, **kwargs) -> bool: ...
-
-    @property
-    def logger(self) -> logging.Logger: ...
-
-    @logger.setter
-    def logger(self, logger: logging.Logger) -> None: ...

src/aind_behavior_services/launcher/app_service.py

@@ -10,25 +10,12 @@
 
 from ._service import IService
 
+logger = logging.getLogger(__name__)
 
-class App(IService):
-    def __init__(self, *args, logger: Optional[logging.Logger] = None, **kwargs) -> None:
-        self._logger = logger
-
-    def validate(self, *args, **kwargs) -> bool:
-        raise NotImplementedError
 
-    @property
-    def logger(self) -> logging.Logger:
-        if self._logger is None:
-            raise ValueError("Logger not set")
-        return self._logger
-
-    @logger.setter
-    def logger(self, logger: logging.Logger) -> None:
-        if self._logger is not None:
-            raise ValueError("Logger already set")
-        self._logger = logger
+class App(IService):
+    # Dummy class for now
+    pass
 
 
 class BonsaiApp(App):
@@ -56,11 +43,9 @@ def __init__(
         additional_properties: Optional[Dict[str, str]] = None,
         cwd: Optional[os.PathLike] = None,
         timeout: Optional[float] = None,
-        logger: Optional[logging.Logger] = None,
         print_cmd: bool = False,
         **kwargs,
     ) -> None:
-        super().__init__(logger)
         self.executable = Path(executable).resolve()
         self.workflow = Path(workflow).resolve()
         self.is_editor_mode = is_editor_mode
@@ -94,8 +79,8 @@ def run(self) -> subprocess.CompletedProcess:
         self.validate()
 
         if self.is_editor_mode:
-            self.logger.warning("Bonsai is running in editor mode. Cannot assert successful completion.")
-        self.logger.info("Bonsai process running...")
+            logger.warning("Bonsai is running in editor mode. Cannot assert successful completion.")
+        logger.info("Bonsai process running...")
         proc = run_bonsai_process(
             workflow_file=self.workflow,
             bonsai_exe=self.executable,
@@ -108,7 +93,7 @@ def run(self) -> subprocess.CompletedProcess:
             print_cmd=self.print_cmd,
         )
         self._result = proc
-        self.logger.info("Bonsai process completed.")
+        logger.info("Bonsai process completed.")
         return proc
 
     def output_from_result(self, allow_stderr: Optional[bool]) -> Self:
@@ -119,13 +104,13 @@ def output_from_result(self, allow_stderr: Optional[bool]) -> Self:
             self._log_process_std_output("Bonsai", proc)
             raise e
         else:
-            self.logger.info("Result from bonsai process is valid.")
+            logger.info("Result from bonsai process is valid.")
             self._log_process_std_output("Bonsai", proc)
 
             if len(proc.stdout) > 0:
-                self.logger.error("Bonsai process finished with errors.")
+                logger.error("Bonsai process finished with errors.")
                 if allow_stderr is None:
-                    allow_stderr = UIHelper(self.logger, print).prompt_yes_no_question(
+                    allow_stderr = UIHelper(logger, print).prompt_yes_no_question(
                         "Would you like to see the error message?"
                     )
                 if allow_stderr is False:
@@ -147,7 +132,7 @@ def prompt_visualizer_layout_input(
         while has_pick is False:
             try:
                 available_layouts.insert(0, "None")
-                picked = UIHelper(self.logger, print).prompt_pick_file_from_list(
+                picked = UIHelper(logger, print).prompt_pick_file_from_list(
                     available_layouts,
                     prompt="Pick a visualizer layout:",
                     zero_label="Default",
@@ -157,13 +142,13 @@ def prompt_visualizer_layout_input(
                 if picked == "None":
                     picked = ""
             except ValueError as e:
-                self.logger.error("Invalid choice. Try again. %s", e)
+                logger.error("Invalid choice. Try again. %s", e)
             has_pick = True
         self.layout = picked
         return self.layout
 
     def _log_process_std_output(self, process_name: str, proc: subprocess.CompletedProcess) -> None:
         if len(proc.stdout) > 0:
-            self.logger.info("%s full stdout dump: \n%s", process_name, proc.stdout)
+            logger.info("%s full stdout dump: \n%s", process_name, proc.stdout)
         if len(proc.stderr) > 0:
-            self.logger.error("%s full stderr dump: \n%s", process_name, proc.stderr)
+            logger.error("%s full stderr dump: \n%s", process_name, proc.stderr)

src/aind_behavior_services/launcher/data_mapper_service.py

@@ -44,33 +44,20 @@
 
 TSession = TypeVar("TSession", bound=AindBehaviorSessionModel)
 
+logger = logging.getLogger(__name__)
 
-class DataMapperService(IService):
-    def __init__(self, *args, logger: Optional[logging.Logger] = None, **kwargs):
-        self._logger = logger
-
-    @property
-    def logger(self) -> logging.Logger:
-        if self._logger is None:
-            raise ValueError("Logger not set")
-        return self._logger
-
-    @logger.setter
-    def logger(self, logger: logging.Logger) -> None:
-        if self._logger is not None:
-            raise ValueError("Logger already set")
-        self._logger = logger
 
-    def validate(self, *args, **kwargs) -> bool:
-        raise NotImplementedError
+class DataMapperService(IService):
+    # Dummy class for now
+    pass
 
 
 class AindDataSchemaSessionDataMapper(DataMapperService):
     def validate(self, *args, **kwargs) -> bool:
         return True
 
     def map_from_session_root(self, *args, **kwargs) -> AdsSession:
-        self.logger.info("Mapping to aind-data-schema Session")
+        logger.info("Mapping to aind-data-schema Session")
         return map_from_session_root(*args, **kwargs)
 
 

src/aind_behavior_services/launcher/logging_helper.py

@@ -10,8 +10,7 @@
 TLogger = TypeVar("TLogger", bound=logging.Logger)
 
 
-def default_logger_factory(output_path: Optional[os.PathLike]) -> logging.Logger:
-    logger = logging.getLogger(__name__)
+def default_logger_builder(logger: TLogger, output_path: Optional[os.PathLike]) -> logging.Logger:
     logger.setLevel(logging.INFO)
     fmt = "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
     utc_formatter = _TzFormatter(fmt, tz=datetime.timezone.utc)

src/aind_behavior_services/launcher/resource_monitor_service.py

@@ -8,29 +8,17 @@
 
 from aind_behavior_services.launcher._service import IService
 
+logger = logging.getLogger(__name__)
+
 
 class ResourceMonitor(IService):
     def __init__(
         self,
         *args,
-        logger: Optional[logging.Logger] = None,
         constrains: Optional[List[Constraint]] = None,
         **kwargs,
     ) -> None:
         self.constraints = constrains or []
-        self._logger = logger
-
-    @property
-    def logger(self) -> logging.Logger:
-        if self._logger is None:
-            raise ValueError("Logger not set")
-        return self._logger
-
-    @logger.setter
-    def logger(self, logger: logging.Logger) -> None:
-        if self._logger is not None:
-            raise ValueError("Logger already set")
-        self._logger = logger
 
     def validate(self, *args, **kwargs) -> bool:
         return self.evaluate_constraints()
@@ -44,7 +32,7 @@ def remove_constraint(self, constraint: Constraint) -> None:
     def evaluate_constraints(self) -> bool:
         for constraint in self.constraints:
             if not constraint():
-                self.logger.error(constraint.on_fail())
+                logger.error(constraint.on_fail())
                 return False
         return True
 

src/aind_behavior_services/launcher/services.py

@@ -6,18 +6,20 @@
 import aind_behavior_services.launcher.watchdog_service as watchdog_service
 from aind_behavior_services.launcher.data_mapper_service import AindDataSchemaSessionDataMapper
 
+logger = logging.getLogger(__name__)
+
 SupportedServices = Union[
     watchdog_service.WatchdogService,
     resource_monitor_service.ResourceMonitor,
     app_service.BonsaiApp,
     AindDataSchemaSessionDataMapper,
 ]
+
 TService = TypeVar("TService", bound=SupportedServices)
 
 
 class Services:
     # todo: This could use some future refactoring to make it more generic. For instance, not subclassing App
-    _logger: Optional[logging.Logger]
     _watchdog: Optional[watchdog_service.WatchdogService]
     _resource_monitor: Optional[resource_monitor_service.ResourceMonitor]
     _app: Optional[app_service.BonsaiApp]
@@ -26,38 +28,16 @@ class Services:
     def __init__(
         self,
         *args,
-        logger: Optional[logging.Logger] = None,
         watchdog: Optional[watchdog_service.WatchdogService] = None,
         resource_monitor: Optional[resource_monitor_service.ResourceMonitor] = None,
         app: Optional[app_service.BonsaiApp] = None,
         data_mapper: Optional[AindDataSchemaSessionDataMapper] = None,
         **kwargs,
     ) -> None:
-        self._logger = logger
         self._watchdog = watchdog
         self._resource_monitor = resource_monitor
         self._app = app
         self._data_mapper = data_mapper
-        if logger is not None:
-            self._register_logger()
-
-    @property
-    def logger(self) -> logging.Logger:
-        if self._logger is None:
-            raise ValueError("Logger not set")
-        return self._logger
-
-    @logger.setter
-    def logger(self, logger: logging.Logger):
-        if self._logger is not None:
-            raise ValueError("Logger already set")
-        self._logger = logger
-        self._register_logger()
-
-    def register_logger(self, logger: logging.Logger) -> Self:
-        # For symmetry with other register methods
-        self.logger = logger
-        return self
 
     @property
     def watchdog(self) -> Optional[watchdog_service.WatchdogService]:
@@ -66,7 +46,7 @@ def watchdog(self) -> Optional[watchdog_service.WatchdogService]:
     def register_watchdog(self, watchdog: watchdog_service.WatchdogService) -> Self:
         if self._watchdog is not None:
             raise ValueError("Watchdog already registered")
-        self._watchdog = self._register_service(watchdog)
+        self._watchdog = watchdog
         return self
 
     @property
@@ -76,7 +56,7 @@ def resource_monitor(self) -> Optional[resource_monitor_service.ResourceMonitor]
     def register_resource_monitor(self, resource_monitor: resource_monitor_service.ResourceMonitor) -> Self:
         if self._resource_monitor is not None:
             raise ValueError("Resource manager already registered")
-        self._resource_monitor = self._register_service(resource_monitor)
+        self._resource_monitor = resource_monitor
         return self
 
     @property
@@ -86,7 +66,7 @@ def app(self) -> Optional[app_service.BonsaiApp]:
     def register_app(self, app: app_service.BonsaiApp) -> Self:
         if self._app is not None:
             raise ValueError("App already registered")
-        self._app = self._register_service(app)
+        self._app = app
         return self
 
     @property
@@ -96,20 +76,10 @@ def data_mapper(self) -> Optional[AindDataSchemaSessionDataMapper]:
     def register_data_mapper(self, data_mapper: AindDataSchemaSessionDataMapper) -> Self:
         if self._data_mapper is not None:
             raise ValueError("Data mapper already registered")
-        self._data_mapper = self._register_service(data_mapper)
+        self._data_mapper = data_mapper
         return self
 
     def validate_service(self, obj: Optional[TService]) -> bool:
         if obj is None:
             raise ValueError("Service not set")
         return obj.validate()
-
-    def _register_service(self, obj: TService, register_logger: bool = True) -> TService:
-        if register_logger:
-            obj.logger = self.logger
-        return obj
-
-    def _register_logger(self) -> None:
-        for service in (self._watchdog, self._resource_monitor, self._app, self._data_mapper):
-            if service is not None:
-                service.logger = self.logger

src/aind_behavior_services/launcher/ui_helper.py

@@ -7,14 +7,14 @@
 from aind_behavior_services.session import AindBehaviorSessionModel
 from aind_behavior_services.task_logic import AindBehaviorTaskLogicModel
 
+logger = logging.getLogger(__name__)
+
 
 class UIHelper:
     _print: Callable[[str], None]
-    _logger: logging.Logger
 
-    def __init__(self, logger: logging.Logger, print_func: Callable[[str], None] = print):
+    def __init__(self, print_func: Callable[[str], None] = print):
         self._print = print_func
-        self._logger = logger
 
     T = TypeVar("T", bound=Any)
 
@@ -66,7 +66,7 @@ def choose_subject(self, subject_list: SubjectDataBase) -> str:
                 if not isinstance(subject, str):
                     raise ValueError("Return value is not a string type.")
             except ValueError as e:
-                self._logger.error("Invalid choice. Try again. %s", e)
+                logger.error("Invalid choice. Try again. %s", e)
         return subject
 
     @staticmethod
@@ -102,4 +102,4 @@ def print_header(
             "-------------------------------"
         )
 
-        self._logger.info(_str)
+        logger.info(_str)