Added generic logging interface (#255)

* Added generic logging interface

* coverage, pr feedback

cumulus_library/actions/builder.py

@@ -10,17 +10,16 @@
 from rich.progress import Progress, TaskID
 
 from cumulus_library import (
-    __version__,
     base_table_builder,
     base_utils,
     databases,
     enums,
     errors,
+    log_utils,
     protected_table_builder,
     study_parser,
 )
 from cumulus_library.statistics import psm
-from cumulus_library.template_sql import base_templates
 
 
 @contextlib.contextmanager
@@ -263,29 +262,14 @@ def run_statistics_builders(
             config=config,
             manifest=manifest,
         )
-
-        insert_query = base_templates.get_insert_into_query(
-            f"{manifest.get_study_prefix()}__{enums.ProtectedTables.STATISTICS.value}",
-            [
-                "study_name",
-                "library_version",
-                "table_type",
-                "table_name",
-                "view_name",
-                "created_on",
-            ],
-            [
-                [
-                    manifest.get_study_prefix(),
-                    __version__,
-                    config_type,
-                    f"{target_table}_{safe_timestamp}",
-                    target_table,
-                    base_utils.get_utc_datetime(),
-                ]
-            ],
+        log_utils.log_statistics(
+            cursor=cursor,
+            schema=schema,
+            manifest=manifest,
+            table_type=config_type,
+            table_name=f"{target_table}_{safe_timestamp}",
+            view_name=target_table,
         )
-        cursor.execute(insert_query)
 
 
 def run_matching_table_builder(

cumulus_library/actions/file_generator.py

@@ -85,7 +85,7 @@ def run_generate_markdown(
         if len(count_tables) > 0:
             f.write(f"## {manifest_parser.get_study_prefix()} count tables\n\n")
             for table in count_tables:
-                manifest_parser._write_md_table(table, study_df, f)
+                _write_md_table(table, study_df, f)
         if len(base_tables) > 0:
             f.write(f"## {manifest_parser.get_study_prefix()} base tables\n\n")
             for table in base_tables:

cumulus_library/cli.py

@@ -17,7 +17,7 @@
     databases,
     enums,
     errors,
-    protected_table_builder,
+    log_utils,
     study_parser,
 )
 from cumulus_library.actions import (
@@ -28,7 +28,6 @@
     importer,
     uploader,
 )
-from cumulus_library.template_sql import base_templates
 
 
 class StudyRunner:
@@ -49,36 +48,6 @@ def get_schema(self, manifest: study_parser.StudyManifestParser):
             return dedicated
         return self.schema_name
 
-    def update_transactions(
-        self, manifest: study_parser.StudyManifestParser, status: str
-    ):
-        """Adds a record to a study's transactions table"""
-        if manifest.get_dedicated_schema():
-            transactions = (
-                f"{manifest.get_dedicated_schema()}."
-                f"{enums.ProtectedTables.TRANSACTIONS.value}"
-            )
-        else:
-            transactions = (
-                f"{manifest.get_study_prefix()}__"
-                f"{enums.ProtectedTables.TRANSACTIONS.value}"
-            )
-        self.cursor.execute(
-            base_templates.get_insert_into_query(
-                transactions,
-                protected_table_builder.TRANSACTIONS_COLS,
-                [
-                    [
-                        manifest.get_study_prefix(),
-                        __version__,
-                        status,
-                        base_utils.get_utc_datetime(),
-                    ]
-                ],
-                {"event_time": "TIMESTAMP"},
-            )
-        )
-
     ### Creating studies
 
     def clean_study(
@@ -153,7 +122,12 @@ def clean_and_build_study(
                 config=config,
             )
             if not continue_from:
-                self.update_transactions(manifest, "started")
+                log_utils.log_transaction(
+                    cursor=self.cursor,
+                    schema=schema,
+                    manifest=manifest,
+                    status=enums.LogStatuses.STARTED,
+                )
                 cleaned_tables = cleaner.clean_study(
                     manifest_parser=manifest,
                     cursor=self.cursor,
@@ -173,7 +147,12 @@ def clean_and_build_study(
                     config=config,
                 )
             else:
-                self.update_transactions(manifest, "resumed")
+                log_utils.log_transaction(
+                    cursor=self.cursor,
+                    schema=schema,
+                    manifest=manifest,
+                    status=enums.LogStatuses.RESUMED,
+                )
             builder.build_study(
                 manifest,
                 self.cursor,
@@ -195,14 +174,24 @@ def clean_and_build_study(
                 verbose=self.verbose,
                 config=config,
             )
-            self.update_transactions(manifest, "finished")
+            log_utils.log_transaction(
+                cursor=self.cursor,
+                schema=schema,
+                manifest=manifest,
+                status=enums.LogStatuses.FINISHED,
+            )
 
         except errors.StudyManifestFilesystemError as e:
             # This should be thrown prior to any database connections, so
             # skipping logging
             raise e
         except Exception as e:
-            self.update_transactions(manifest, "error")
+            log_utils.log_transaction(
+                cursor=self.cursor,
+                schema=schema,
+                manifest=manifest,
+                status=enums.LogStatuses.ERROR,
+            )
             raise e
 
     def run_matching_table_builder(

cumulus_library/enums.py

@@ -1,18 +1,28 @@
 """Holds enums used across more than one module"""
 
-from enum import Enum
+import enum
 
 
-class ProtectedTableKeywords(Enum):
+class ProtectedTableKeywords(enum.Enum):
     """Tables with a pattern like '_{keyword}_' are not manually dropped."""
 
     ETL = "etl"
     LIB = "lib"
     NLP = "nlp"
 
 
-class ProtectedTables(Enum):
+class ProtectedTables(enum.Enum):
     """Tables created by cumulus for persistence outside of study rebuilds"""
 
     STATISTICS = "lib_statistics"
     TRANSACTIONS = "lib_transactions"
+
+
+class LogStatuses(enum.Enum):
+    DEBUG = "debug"
+    ERROR = "error"
+    FINISHED = "finished"
+    INFO = "info"
+    RESUMED = "resumed"
+    STARTED = "started"
+    WARN = "warn"

cumulus_library/log_utils.py

@@ -0,0 +1,95 @@
+"""A set of convenience functions for database logging"""
+
+from cumulus_library import (
+    __version__,
+    base_utils,
+    databases,
+    enums,
+    errors,
+    study_parser,
+)
+from cumulus_library.template_sql import base_templates, sql_utils
+
+
+def log_transaction(
+    *,
+    cursor: databases.DatabaseCursor,
+    schema: str,
+    manifest: study_parser.StudyManifestParser,
+    status: enums.LogStatuses | str | None = enums.LogStatuses.INFO,
+    message: str | None = None,
+):
+    if isinstance(status, str):
+        try:
+            status = enums.LogStatuses(status)
+        except ValueError as e:
+            raise errors.CumulusLibraryError(
+                f"Invalid event type {status} requested for transaction log.\n"
+                f"Valid types: {','.join([x.value for x in enums.LogStatuses])}"
+            ) from e
+    _log_table(
+        table=sql_utils.TransactionsTable(),
+        cursor=cursor,
+        schema=schema,
+        manifest=manifest,
+        dataset=[
+            [
+                manifest.get_study_prefix(),
+                __version__,
+                status.value,
+                base_utils.get_utc_datetime(),
+                message or None,
+            ]
+        ],
+    )
+
+
+def log_statistics(
+    *,
+    cursor: databases.DatabaseCursor,
+    schema: str,
+    manifest: study_parser.StudyManifestParser,
+    table_type: str,
+    table_name: str,
+    view_name: str,
+):
+    _log_table(
+        table=sql_utils.StatisticsTable(),
+        cursor=cursor,
+        schema=schema,
+        manifest=manifest,
+        dataset=[
+            [
+                manifest.get_study_prefix(),
+                __version__,
+                table_type,
+                table_name,
+                view_name,
+                base_utils.get_utc_datetime(),
+            ]
+        ],
+    )
+
+
+def _log_table(
+    *,
+    table: sql_utils.BaseTable,
+    cursor: databases.DatabaseCursor,
+    schema: str,
+    manifest: study_parser.StudyManifestParser,
+    dataset: list[list],
+):
+    if manifest and manifest.get_dedicated_schema():
+        db_schema = manifest.get_dedicated_schema()
+        table_name = table.name
+    else:
+        db_schema = schema
+        table_name = f"{manifest.get_study_prefix()}__{table.name}"
+    query = base_templates.get_insert_into_query(
+        schema=db_schema,
+        table_name=table_name,
+        table_cols=table.columns,
+        dataset=dataset,
+        type_casts=table.type_casts,
+    )
+    cursor.execute(query)

cumulus_library/protected_table_builder.py

@@ -1,10 +1,10 @@
 """Builder for creating tables for tracking state/logging changes"""
 
-from cumulus_library import base_table_builder, enums, study_parser
+from cumulus_library import base_table_builder, databases, enums, study_parser
 from cumulus_library.template_sql import base_templates
 
-TRANSACTIONS_COLS = ["study_name", "library_version", "status", "event_time"]
-TRANSACTION_COLS_TYPES = ["varchar", "varchar", "varchar", "timestamp"]
+TRANSACTIONS_COLS = ["study_name", "library_version", "status", "event_time", "message"]
+TRANSACTION_COLS_TYPES = ["varchar", "varchar", "varchar", "timestamp", "varchar"]
 # while it may seem redundant, study_name and view_name are included as a column for
 # ease of constructing a view of multiple transaction tables
 STATISTICS_COLS = [
@@ -32,7 +32,7 @@ class ProtectedTableBuilder(base_table_builder.BaseTableBuilder):
 
     def prepare_queries(
         self,
-        cursor: object,
+        cursor: databases.DatabaseCursor,
         schema: str,
         study_name: str,
         study_stats: dict,

cumulus_library/template_sql/base_templates.py

@@ -298,6 +298,7 @@ def get_extension_denormalize_query(config: sql_utils.ExtensionConfig) -> str:
 
 
 def get_insert_into_query(
+    schema: str,
     table_name: str,
     table_cols: list[str],
     dataset: list[list[str]],
@@ -313,6 +314,7 @@ def get_insert_into_query(
     type_casts = type_casts or {}
     return get_base_template(
         "insert_into",
+        schema_name=schema,
         table_name=table_name,
         table_cols=table_cols,
         dataset=dataset,

cumulus_library/template_sql/insert_into.sql.jinja

@@ -1,5 +1,5 @@
 {%- import 'syntax.sql.jinja' as syntax -%}
-INSERT INTO {{ table_name }}
+INSERT INTO {{ schema_name }}.{{ table_name }}
 (
     {%- for col in table_cols -%}
     "{{ col }}"
@@ -12,6 +12,8 @@ VALUES
     {%- for field in row -%}
     {%- if table_cols[loop.index0] in type_casts.keys() -%}
     {{ type_casts[table_cols[loop.index0]] }} '{{ field }}'
+    {%- elif not field -%}
+    CAST(NULL AS VARCHAR)
     {%- else -%}
     '{{ field }}'
     {%- endif -%}