Merge pull request #17 from moka-guys/development

Fix logging (#17)

Co-Authored-By: Graeme <graeme_c_smith@hotmail.com>
Co-Authored-By: rebeccahaines1 <84131466+rebeccahaines1@users.noreply.github.com>

Author: Graeme-Smith

samplesheet_validator/__main__.py

@@ -1,7 +1,10 @@
 import os
+import sys
+import logging
 import argparse
 from .samplesheet_validator import SamplesheetCheck
 from .ss_logger import set_root_logger
+from .config import LOGGING_FORMATTER
 
 
 def get_arguments():
@@ -98,10 +101,10 @@ def is_valid_dir(parser: argparse.ArgumentParser, dir: str) -> str:
         return dir
 
 
+
 if __name__ == "__main__":
     parsed_args = get_arguments()
-    if not parsed_args.no_stream_handler:
-        set_root_logger()  # Adds stream handler
+    logger = set_root_logger(parsed_args.no_stream_handler)
     sscheck_obj = SamplesheetCheck(
         parsed_args.samplesheet_path,
         parsed_args.sequencer_ids,
@@ -111,3 +114,4 @@ def is_valid_dir(parser: argparse.ArgumentParser, dir: str) -> str:
         parsed_args.logdir,
     )
     sscheck_obj.ss_checks()  # Carry out samplesheeet validation
+

samplesheet_validator/config.py

@@ -3,7 +3,7 @@
 TIMESTAMP = str(f"{datetime.datetime.now():%Y%m%d_%H%M%S}")
 
 # Specifies the layout of log records in the final output
-LOGGING_FORMATTER = "%(asctime)s - SAMPLESHEET_VALIDATOR - %(levelname)s - %(message)s"
+LOGGING_FORMATTER = "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
 
 LOG_MSGS = {
     "ss_present": "Samplesheet with supplied name exists (%s)",
@@ -14,11 +14,11 @@
     "sequencer_id_invalid": "Sequencer id not in allowed list (%s, %s)",
     "ss_not_empty": "Samplesheet is (>10 bytes)",
     "ss_empty": "Samplesheet empty (<10 bytes)",
-    "found_header_line": "Line in samplesheet identified as a header line",
-    "found_sample_line": "Line in samplesheet identified as containing a sample",
-    "error_extracting_headers": "An error was encountered when extracting headers from the samplesheet: %s",
-    "found_empty_line": "Line in samplesheet is an empty line",
-    "col_extraction_error": "Exception raised while attempting to extract %s from sample line %s: %s",
+    "found_header_line": "Line %s in samplesheet identified as a header line",
+    "found_sample_line": "Line %s in samplesheet identified as containing a sample",
+    "error_extracting_headers": "An error was encountered when extracting headers from the samplesheet, from line %s: %s",
+    "found_empty_line": "Line %s in samplesheet is an empty line",
+    "col_extraction_error": "Exception raised while attempting to extract %s from sample line %s, %s: %s",
     "headers_as_expected": "Expected headers present in samplesheet",
     "headers_err": "Header(/s) missing from [Data] section: '%s'",
     "samplenames_match": "All sample names and sample IDS match",

samplesheet_validator/samplesheet_validator.py

@@ -12,6 +12,7 @@
 """
 import os
 import re
+import logging
 from typing import Union
 from . import config
 from .ss_logger import SSLogger
@@ -42,6 +43,9 @@ class SamplesheetCheck:
         panels (list):                  Valid pan numbers
         tso_panels (list):              Valid TSO pannumbers
         development_panels (list):      Development pan numbers
+        runfolder_name (str):           Name of runfolder
+        logfile_path (str):             Path to use for logfile
+        logger (logging.Logger):        Logger                      
 
     Methods:
         get_logger()
@@ -115,24 +119,24 @@ def __init__(
         self.panels = panels
         self.tso_panels = tso_panels
         self.dev_panno = dev_panno
+        self.runfolder_name = (self.samplesheet_path.split("/")[-1]).split("_SampleSheet.csv")[0]
+        self.logfile_path = f"{os.path.join(logdir, self.runfolder_name)}_samplesheet_validator.log"
+        self.logger = self.get_logger()
+
 
-    def get_logger(self):
+    def get_logger(self) -> logging.Logger:
         """
         Get logger for the class
-            :return (object):    Logger
+            :return (object):   Logger
         """
-        runfolder_name = (self.samplesheet_path.split("/")[-1]).split(
-            "_SampleSheet.csv"
-        )[0]
-        logfile_path = f"{os.path.join(self.logdir, runfolder_name)}_{config.TIMESTAMP}_samplesheet_validator.log"
-        return SSLogger(logfile_path).get_logger()
+        return SSLogger(self.logfile_path, self.runfolder_name).get_logger(__name__)
+
 
     def ss_checks(self) -> None:
         """
         Run checks at samplesheet and sample level. Performs required extra checks for
         checks not included in seglh-naming
         """
-        self.logger = self.get_logger()
         if self.check_ss_present():
             setattr(self, "ss_obj", self.check_ss_name())
             if self.ss_obj:
@@ -274,57 +278,57 @@ def get_data_section(self) -> None:
             :return None:
         """
         with open(self.samplesheet_path, "r") as samplesheet_stream:
-            for line in reversed(samplesheet_stream.readlines()):
+            samplesheet_contents = samplesheet_stream.readlines()
+            for line in reversed(samplesheet_contents):
+                line_index = samplesheet_contents.index(line)
                 # If line contains table headers, stop looping through the file
                 if any(header in line for header in self.expected_data_headers):
-                    self.extract_headers(line)
+                    self.extract_headers(line, line_index)
                     break
                 elif len(line.split(",")[0]) < 2:
-                    self.logger.info(self.logger.log_msgs["found_empty_line"])
+                    self.logger.info(self.logger.log_msgs["found_empty_line"], line_index)
                     pass  # Skip empty lines
                 else:  # Contains sample
-                    self.extract_sample_name_id(line)
+                    self.extract_sample_name_id(line, line_index)
 
-    def extract_headers(self, line: str) -> None:
+    def extract_headers(self, line: str, line_index: int) -> None:
         """
         Extract headers from line
-            :param line (str):  Line containing samplesheet headers
+            :param line (str):          Line containing samplesheet headers
+            :param line_index (int):    Index of line
         """
         try:
-            self.logger.info(self.logger.log_msgs["found_header_line"])
+            self.logger.info(self.logger.log_msgs["found_header_line"], line_index)
             self.data_headers = line.split(",")
         except Exception as exception:
             self.errors = True
             self.logger.warning(
-                self.logger.log_msgs["error_extracting_headers"], exception
+                self.logger.log_msgs["error_extracting_headers"], line_index, exception
             )
             self.add_msg_to_error_dict(
                 "Error extracting headers",
-                self.logger.log_msgs["error_extracting_headers"] % exception,
+                self.logger.log_msgs["error_extracting_headers"] % (line_index, exception),
             )
 
-    def extract_sample_name_id(self, line: str) -> None:
+    def extract_sample_name_id(self, line: str, line_index: int) -> None:
         """
         Extract sample name and sample id from samplesheet line
             :param line (str):  Line containing sample details
+            :param line_index (int):    Index of line
         """
-        self.logger.info(self.logger.log_msgs["found_sample_line"])
+        self.logger.info(self.logger.log_msgs["found_sample_line"], line_index)
         for column_details in [("Sample_ID", 0), ("Sample_Name", 1)]:
             col_name, index = column_details
             try:
                 self.samples[col_name].append(line.split(",")[index])
             except Exception as exception:
                 self.errors = True
                 self.logger.warning(
-                    self.logger.log_msgs["col_extraction_error"],
-                    col_name,
-                    line,
-                    exception,
+                    self.logger.log_msgs["col_extraction_error"], col_name, line_index, line, exception,
                 )
                 self.add_msg_to_error_dict(
                     "Error extracting sample name and ID",
-                    self.logger.log_msgs["col_extraction_error"]
-                    % (col_name, line, exception),
+                    self.logger.log_msgs["col_extraction_error"] % (col_name, line_index, line, exception)
                 )
 
     def check_expected_headers(self) -> None:

samplesheet_validator/ss_logger.py

@@ -8,59 +8,68 @@
 import logging.handlers
 
 
-def set_root_logger():
+def set_root_logger(no_stream_handler: bool):
     """
-    Set up root logger and add stream handler - we only want to add stream handler once
-    else it will duplicate log messages to the terminal
+    Set up root logger and add stream handler and syslog handler - we only want to add these once
+    else it will duplicate log messages to the terminal. All loggers named with the same stem
+    as the root logger will use these same syslog handler and stream handler
+        :param no_stream_handler (bool):    True if no stream handler specified as command line input
     """
+    formatter = logging.Formatter(config.LOGGING_FORMATTER)
     logger = logging.getLogger()
     logger.setLevel(logging.DEBUG)
-    stream_handler = logging.StreamHandler(sys.stdout)
-    stream_handler.setLevel(logging.DEBUG)
-    stream_handler.setFormatter(logging.Formatter(config.LOGGING_FORMATTER))
-    stream_handler.name = "stream_handler"
-    logger.addHandler(stream_handler)
+    syslog_handler = logging.handlers.SysLogHandler(address="/dev/log")
+    syslog_handler.setFormatter(formatter)
+    syslog_handler.name = "syslog_handler"
+    logger.addHandler(syslog_handler)
+    if not no_stream_handler:
+        stream_handler = logging.StreamHandler(sys.stdout)
+        stream_handler.setFormatter(formatter)
+        stream_handler.name = "stream_handler"
+        logger.addHandler(stream_handler)
+    return logger
+
 
 
 class SSLogger:
     """
     Creates a python logging object with a file handler and syslog handler
 
     Attributes
-        timestamp (str):                        Timestamp from config
         logfile_path (str):                     Name of filepath to provide to _file_handler()
+        runfolder_name (str):                   Runfolder name
         logging_formatter (logging.Formatter):  Specifies the layout of log records in the final output
 
     Methods
-        get_logger()
+        get_logger(logger_name)
             Returns a Python logging object
         _get_file_handler()
             Get file handler for the logger
         _get_syslog_handler()
             Get syslog handler for the logger
     """
 
-    def __init__(self, logfile_path: str):
+    def __init__(self, logfile_path: str, runfolder_name: str):
         """
         Constructor for the Logger class
             :param logfile_path (str):      Path to logfile location
+            :param runfolder_name (str):    Runfolder name
         """
         # Timestamp used for naming log files with datetime, format %Y%m%d_%H%M%S
-        self.timestamp = config.TIMESTAMP
         self.logfile_path = logfile_path
+        self.runfolder_name = runfolder_name
         self.logging_formatter = logging.Formatter(config.LOGGING_FORMATTER)
 
-    def get_logger(self) -> logging.Logger:
+    def get_logger(self, logger_name: str) -> logging.Logger:
         """
         Returns a Python logging object, and give it a name
+            :param logger_name (str):   Logger name string
             :return logger (object):    Python logging object with custom attributes
         """
-        logger = logging.getLogger()
+        logger = logging.getLogger(f"{logger_name}.{self.runfolder_name}")
         logger.filepath = self.logfile_path
         logger.setLevel(logging.DEBUG)
         logger.addHandler(self._get_file_handler())
-        logger.addHandler(self._get_syslog_handler())
-        logger.timestamp = self.timestamp
         logger.log_msgs = config.LOG_MSGS
         return logger
 
@@ -74,14 +83,3 @@ def _get_file_handler(self) -> logging.FileHandler:
         file_handler.setFormatter(self.logging_formatter)
         file_handler.name = "file_handler"
         return file_handler
-
-    def _get_syslog_handler(self) -> logging.handlers.SysLogHandler:
-        """
-        Get syslog handler for the logger, and give it a name
-            :return syslog_handler (logging.SysLogHandler): SysLogHandler
-        """
-        syslog_handler = logging.handlers.SysLogHandler(address="/dev/log")
-        syslog_handler.setLevel(logging.DEBUG)
-        syslog_handler.setFormatter(self.logging_formatter)
-        syslog_handler.name = "syslog_handler"
-        return syslog_handler

setup.py

@@ -3,7 +3,7 @@
 
 setup(
     name="samplesheet_validator",
-    version=git_tag(),
+    version="TEST",
     description="Python library for samplesheet validation",
     url="https://github.com/moka-guys/samplesheet_validator",
     author="Rachel Duffin",

test/test_samplesheet_validator.py

@@ -904,7 +904,7 @@ def test_extract_sample_name_id_pass(self, valid_samplesheets_with_dev, caplog):
         for samplesheet in valid_samplesheets_with_dev:
             sscheck_obj = get_sscheck_obj(samplesheet)
             assert (
-                "Line in samplesheet identified as containing a sample" in caplog.text
+                "samplesheet identified as containing a sample" in caplog.text
             )
             shutdown_logs(sscheck_obj.logger)