Adding logpath to data plugins collector

nodescraper/base/inbandcollectortask.py

@@ -57,6 +57,22 @@ def __init__(
         session_id: Optional[str] = None,
         **kwargs,
     ):
+        """Creates a InBandDataCollector Class
+
+        Args:
+            system_info (SystemInfo): Information about the system being targeted for data collection.
+            connection (InBandConnection): The in-band connection used to communicate with the target system.
+            logger (Optional[logging.Logger], optional): Logger instance for logging messages. Defaults to None.
+            system_interaction_level (SystemInteractionLevel, optional): Level of interaction with the system. Defaults to SystemInteractionLevel.INTERACTIVE.
+            max_event_priority_level (Union[EventPriority, str], optional): Maximum priority level for events. Defaults to EventPriority.CRITICAL.
+            parent (Optional[str], optional): Parent task identifier. Defaults to None.
+            task_result_hooks (Optional[list[TaskResultHook]], optional): List of hooks to be called with task results. Defaults to None.
+            event_reporter (str, optional): Event reporter identifier. Defaults to DEFAULT_EVENT_REPORTER.
+            session_id (Optional[str], optional): Session identifier for the data collection task. Defaults to None.
+
+        Raises:
+            SystemCompatibilityError: Raised if the system's OS family is not supported.
+        """
         super().__init__(
             system_info=system_info,
             system_interaction_level=system_interaction_level,

nodescraper/base/redfishcollectortask.py

@@ -52,6 +52,18 @@ def __init__(
         session_id: Optional[str] = None,
         **kwargs,
     ):
+        """Creates a RedfishDataCollector instance.
+
+        Args:
+            system_info (SystemInfo): system info object for target system for data collection
+            connection (TConnection): connection object for the data collector
+            logger (Optional[logging.Logger], optional): python logger object. Defaults to None.
+            max_event_priority_level (Union[EventPriority, str], optional): priority limit for events. Defaults to EventPriority.CRITICAL.
+            parent (Optional[str], optional): parent task identifier. Defaults to None.
+            task_result_hooks (Optional[list[TaskResultHook]], optional): list of task result hooks. Defaults to None.
+            event_reporter (str, optional): Reporter string stored on emitted events. Defaults to DEFAULT_EVENT_REPORTER.
+            session_id (Optional[str], optional): session identifier. Defaults to None.
+        """
         super().__init__(
             system_info=system_info,
             connection=connection,

nodescraper/connection/inband/inbandmanager.py

@@ -56,6 +56,17 @@ def __init__(
         connection_args: Optional[SSHConnectionParams | dict[str, Any]] = None,
         **kwargs,
     ):
+        """Initialize a new InBandConnectionManager instance.
+
+        Args:
+            system_info (SystemInfo): System info for the targeted system
+            logger (Optional[Logger], optional): Logger for the connection manager. Defaults to None.
+            max_event_priority_level (Union[EventPriority, str], optional): Maximum event priority level for logging. Defaults to EventPriority.CRITICAL.
+            parent (Optional[str], optional): Parent task identifier. Defaults to None.
+            task_result_hooks (Optional[list[TaskResultHook]], optional): List of task result hooks. Defaults to None.
+            connection_args (Optional[SSHConnectionParams], optional): SSH connection parameters
+                These connections args will be used to establish the SSH connection to the targeted system. Defaults to None.
+        """
         super().__init__(
             system_info,
             logger,

nodescraper/connection/inband/inbandremote.py

@@ -55,6 +55,11 @@ def __init__(
         self,
         ssh_params: SSHConnectionParams,
     ) -> None:
+        """Creates a RemoteShell Instance.
+
+        Args:
+            ssh_params (SSHConnectionParams): The parameters used to initiate the SSH connection username/password/ect.
+        """
         self.ssh_params = ssh_params
         self.client = paramiko.SSHClient()
         self.client.load_system_host_keys()

nodescraper/connection/redfish/redfish_connection.py

@@ -92,13 +92,24 @@ def __init__(
         verify_ssl: bool = True,
         api_root: Optional[str] = None,
     ):
-        self.base_url = base_url.rstrip("/")
-        self.api_root = (api_root or DEFAULT_REDFISH_API_ROOT).strip("/")
-        self.username = username
-        self.password = password or ""
-        self.timeout = timeout
-        self.use_session_auth = use_session_auth
-        self.verify_ssl = verify_ssl
+        """Creates a RedfishConnection instance.
+
+        Args:
+            base_url (str): The base url for the redfish service typically this is /redfish/v1
+            username (str): The username for the redfish service.
+            password (Optional[str], optional): The password for the redfish service. Defaults to None.
+            timeout (float, optional): The timeout for redfish requests in seconds. Defaults to 10.0.
+            use_session_auth (bool, optional): Whether to use session-based authentication. Defaults to True.
+            verify_ssl (bool, optional): Whether to verify the SSL certificate. Defaults to True.
+            api_root (Optional[str], optional): The API root for the redfish services. Defaults to None.
+        """
+        self.base_url: str = base_url.rstrip("/")
+        self.api_root: str = (api_root or DEFAULT_REDFISH_API_ROOT).strip("/")
+        self.username: str = username
+        self.password: str = password or ""
+        self.timeout: float = timeout
+        self.use_session_auth: bool = use_session_auth
+        self.verify_ssl: bool = verify_ssl
         self._session: Optional[requests.Session] = None
         self._session_token: Optional[str] = None
         self._session_uri: Optional[str] = None  # For logout DELETE
@@ -133,7 +144,10 @@ def _transport_error_message(self, exc: Exception) -> str:
     def _execute_request(self, request_fn: Callable[[], _T]) -> _T:
         try:
             return request_fn()
-        except (requests.exceptions.Timeout, requests.exceptions.ConnectionError) as exc:
+        except (
+            requests.exceptions.Timeout,
+            requests.exceptions.ConnectionError,
+        ) as exc:
             raise RedfishConnectionError(self._transport_error_message(exc)) from exc
         except socket.gaierror as exc:
             raise RedfishConnectionError(self._transport_error_message(exc)) from exc

nodescraper/connection/redfish/redfish_manager.py

@@ -58,6 +58,20 @@ def __init__(
         connection_args: Optional[RedfishConnectionParams] = None,
         **kwargs,
     ):
+        """Creates a RedfishConnectionManager Task instance.
+
+        Args:
+            system_info (SystemInfo): System information for the connection manager task.
+            logger (Optional[logging.Logger], optional): _description_. Defaults to None.
+            max_event_priority_level (Union[EventPriority, str], optional): _description_. Defaults to EventPriority.CRITICAL.
+            parent (Optional[str], optional): _description_. Defaults to None.
+            task_result_hooks (Optional[list[TaskResultHook], None], optional): _description_. Defaults to None.
+            connection_args (Optional[Union[TConnectArg, dict]], optional): _description_. Defaults to None.
+            kwargs (dict, optional): Additional keyword arguments passed to the parent class.
+
+        Raises:
+            ValueError: Will raise a ValueError when the connection_args cannot be mapped to the expected model.
+        """
         super().__init__(
             system_info,
             logger,

nodescraper/interfaces/connectionmanager.py

@@ -101,6 +101,23 @@ def __init__(
         session_id: Optional[str] = None,
         **kwargs,
     ):
+        """Creates a Connection Manager Task instance.
+
+        Args:
+            system_info (SystemInfo): System information for the connection manager task.
+            logger (Optional[logging.Logger], optional): Logger instance for the connection manager task. Defaults to None.
+            max_event_priority_level (Union[EventPriority, str], optional): Maximum priority level for events. Defaults to EventPriority.CRITICAL.
+            parent (Optional[str], optional): Parent task identifier. Defaults to None.
+            task_result_hooks (Optional[list[TaskResultHook], None], optional): List of task result hooks to be executed. Defaults to None.
+            connection_args (Optional[Union[TConnectArg, dict]], optional): Connection arguments for the connection manager.
+                If a dictionary is provided, it will be transformed into a BaseModel instance of the expected type. Defaults to None.
+                When it is None then connection args will be None and it will not be built into the default connection model.
+            event_reporter (str, optional): Event reporter identifier. Defaults to DEFAULT_EVENT_REPORTER.
+            session_id (Optional[str], optional): Session identifier for the connection manager task. Defaults to None.
+
+        Raises:
+            ValueError: Will raise a ValueError when the connection_args cannot be mapped to the expected model.
+        """
         super().__init__(
             system_info=system_info,
             logger=logger,

nodescraper/interfaces/datacollectortask.py

@@ -49,7 +49,9 @@
 from .taskresulthook import TaskResultHook
 
 
-def _supported_sku_name_set(supported: Optional[set[Union[str, Enum]]]) -> Optional[set[str]]:
+def _supported_sku_name_set(
+    supported: Optional[set[Union[str, Enum]]],
+) -> Optional[set[str]]:
     """Map ``SUPPORTED_SKUS`` to string names for comparison with ``SystemInfo.sku``."""
     if not supported:
         return None
@@ -156,15 +158,22 @@ def __init__(
         task_result_hooks: Optional[list[TaskResultHook]] = None,
         event_reporter: str = DEFAULT_EVENT_REPORTER,
         session_id: Optional[str] = None,
+        log_path: Optional[str] = None,
         **kwargs,
     ):
         """data collector init function
 
         Args:
             system_info (SystemInfo): system info object for target system for data collection
-            system_interaction (SystemInteraction): enum to indicate the type of actions that can be performed when interacting with the system
-            event_reporter (str, optional): Reporter string stored on emitted events. Defaults to DEFAULT_EVENT_REPORTER.
+            connection (TConnection): connection object for the data collector
             logger (Optional[logging.Logger], optional): python logger object. Defaults to None.
+            system_interaction_level (SystemInteractionLevel | str): The interaction level which the collector will use
+                determine which commands it can run and how invasive the interactions can be when running those commands.Defaults to SystemInteractionLevel.INTERACTIVE.
+            max_event_priority_level (Union[EventPriority, str], optional): priority limit for events. Defaults to EventPriority.CRITICAL.
+            parent (Optional[str], optional): parent task identifier. Defaults to None.
+            task_result_hooks (Optional[list[TaskResultHook]], optional): list of task result hooks. Defaults to None.
+            event_reporter (str, optional): Reporter string stored on emitted events. Defaults to DEFAULT_EVENT_REPORTER.
+            session_id (Optional[str], optional): session identifier. Defaults to None.
             log_path (Optional[str], optional): file system log path. Defaults to None.
         """
         super().__init__(
@@ -176,13 +185,13 @@ def __init__(
             event_reporter=event_reporter,
             session_id=session_id,
         )
-
         if isinstance(system_interaction_level, str):
             system_interaction_level = getattr(SystemInteractionLevel, system_interaction_level)
 
-        self.system_interaction_level = system_interaction_level
-        self.connection = connection
-        self._html_view = False
+        self.log_path: str | None = log_path
+        self.system_interaction_level: SystemInteractionLevel = system_interaction_level
+        self.connection: TConnection = connection
+        self._html_view: bool = False
 
         allowed_skus = _supported_sku_name_set(self.SUPPORTED_SKUS)
         if (
@@ -230,6 +239,10 @@ def collect_data(
     ) -> tuple[TaskResult, Optional[TDataModel]]:
         """Collect data from a target system
 
+        Args:
+            args (Optional[TCollectArg], optional): collection arguments for this data collection run. Defaults to None.
+
         Returns:
-            tuple[TaskResult, DataModel]: tuple containing result and data model
+            tuple[TaskResult, Optional[TDataModel]]: tuple containing result and data model. No DataModel for collection failure cases therefore
+            it is optional.
         """

nodescraper/interfaces/dataplugin.py

@@ -46,7 +46,7 @@
     SystemInfo,
     TaskResult,
 )
-from nodescraper.utils import pascal_to_snake
+from nodescraper.utils import pascal_to_snake, resolve_log_dir_name
 
 from .connectionmanager import TConnectArg, TConnectionManager
 from .task import SystemCompatibilityError
@@ -94,6 +94,19 @@ def __init__(
         session_id: Optional[str] = None,
         **kwargs,
     ):
+        """_summary_
+
+        Args:
+            system_info (Optional[SystemInfo], optional): system info object. Defaults to None.
+            logger (Optional[logging.Logger], optional): python logger instance. Defaults to None.
+            connection_manager (Optional[TConnectionManager], optional): connection manager instance. Defaults to None.
+            connection_args (Optional[Union[TConnectArg  , dict]], optional): connection args. Defaults to None.
+            task_result_hooks (Optional[list[TaskResultHook]], optional): list of task result hooks. Defaults to None.
+            log_path (Optional[str], optional): path for file system logs. Defaults to None.
+            event_reporter (str, optional): Reporter string stored on emitted events. Defaults to DEFAULT_EVENT_REPORTER.
+            session_id (Optional[str], optional): session identifier. Defaults to None.
+            kwargs (optional): additional keyword arguments. These given to the baseclass as well.
+        """
         super().__init__(
             system_info,
             logger,
@@ -357,15 +370,24 @@ def collect(
 
                 for collector_cls in collector_classes:
                     collector_args = self._resolve_collector_args(collector_cls, collection_args)
-                    collection_task = collector_cls(
+                    collector_log_path = None
+                    if self.log_path:
+                        _log_path = (
+                            Path(self.log_path)
+                            / resolve_log_dir_name(self.__class__.__name__)
+                            / resolve_log_dir_name(collector_cls.__name__)
+                        )
+                        _log_path.mkdir(parents=True, exist_ok=True)
+                        collector_log_path = str(_log_path)
+                    collection_task: DataCollector[Any, Any, Any] = collector_cls(
                         system_info=self.system_info.model_copy(),
                         logger=self.logger,
                         system_interaction_level=system_interaction_level,
                         connection=self.connection_manager.connection,
                         max_event_priority_level=max_event_priority_level,
                         parent=self.__class__.__name__,
                         task_result_hooks=self.task_result_hooks,
-                        log_path=self.log_path,
+                        log_path=collector_log_path,
                         event_reporter=self.event_reporter,
                         session_id=self.session_id,
                     )

nodescraper/interfaces/plugin.py

@@ -64,6 +64,9 @@ def __init__(
             task_result_hooks (Optional[list[TaskResultHook]], optional): list of task result hooks. Defaults to None.
             log_path (Optional[str], optional): path for file system logs. Defaults to None.
             queue_callback (Optional[Callable], optional): function to add additional plugins to plugin executor queue. Defaults to None.
+            event_reporter (str, optional): Reporter string stored on emitted events. Defaults to DEFAULT_EVENT_REPORTER.
+            session_id (Optional[str], optional): session identifier. Defaults to None.
+            kwargs (optional): additional keyword arguments. These are currently not used by the base plugin interface.
         """
         if logger is None:
             logger = logging.getLogger(DEFAULT_LOGGER)

nodescraper/interfaces/resultcollator.py

@@ -25,7 +25,8 @@
 ###############################################################################
 import abc
 import logging
-from typing import Optional
+from logging import Logger
+from typing import Optional, Union
 
 from nodescraper.constants import DEFAULT_LOGGER
 from nodescraper.models import PluginResult, TaskResult
@@ -39,14 +40,23 @@ def __init__(
         logger: Optional[logging.Logger] = None,
         log_path: Optional[str] = None,
     ):
+        """Creates a PluginResultCollator instance.
+
+        Args:
+            logger (Optional[logging.Logger], optional): logger instance for the collator. Defaults to None.
+            log_path (Optional[str], optional): path to the log file. Defaults to None.
+        """
         if logger is None:
             logger = logging.getLogger(DEFAULT_LOGGER)
-        self.logger = logger
-        self.log_path = log_path
+        self.logger: Logger = logger
+        self.log_path: Union[str, None] = log_path
 
     @abc.abstractmethod
     def collate_results(
-        self, plugin_results: list[PluginResult], connection_results: list[TaskResult], **kwargs
+        self,
+        plugin_results: list[PluginResult],
+        connection_results: list[TaskResult],
+        **kwargs,
     ):
         """Function to process the result of a plugin executor run