From 0f1003c48b49c9177fcdd9acbcf8c3a3ec2942a1 Mon Sep 17 00:00:00 2001
From: Christopher Rhodes <christopher.rhodes@embl.de>
Date: Fri, 22 Mar 2024 12:07:39 +0100
Subject: [PATCH] Mocked up session logging implementation using python log
 service; two tests are failing apparently due to Session singleton-related
 issues

---
 model_server/base/session.py | 32 +++++++++++++++++++++-----------
 tests/test_session.py        | 17 ++++++++++++++---
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/model_server/base/session.py b/model_server/base/session.py
index 1b91b12a..ca119b54 100644
--- a/model_server/base/session.py
+++ b/model_server/base/session.py
@@ -1,4 +1,5 @@
 import json
+import logging
 import os
 
 from pathlib import Path
@@ -12,6 +13,8 @@ from model_server.base.workflows import WorkflowRunRecord
 def create_manifest_json():
     pass
 
+logger = logging.getLogger(__name__)
+
 class Session(object):
     """
     Singleton class for a server session that persists data between API calls
@@ -27,8 +30,12 @@ class Session(object):
         self.models = {} # model_id : model object
         self.manifest = [] # paths to data as well as other metadata from each inference run
         self.paths = self.make_paths(root)
-        self.session_log = self.paths['logs'] / f'session.log'
-        self.log_event('Initialized session')
+
+        self.logfile = self.paths['logs'] / f'session.log'
+        logging.basicConfig(filename=self.logfile, level=logging.INFO)
+
+        self.log_info('Initialized session')
+
         self.manifest_json = self.paths['logs'] / f'manifest.json'
         open(self.manifest_json, 'w').close() # instantiate empty json file
 
@@ -75,19 +82,22 @@ class Session(object):
             idx += 1
         return f'{yyyymmdd}-{idx:04d}'
 
-    def log_event(self, event: str):
-        """
-        Write an event string to this session's log file.
-        """
-        timestamp = strftime('%m/%d/%Y, %H:%M:%S', localtime())
-        with open(self.session_log, 'a') as fh:
-            fh.write(f'{timestamp} -- {event}\n')
+    def log_info(self, msg):
+        logger.info(msg)
+
+    # def log_event(self, event: str):
+    #     """
+    #     Write an event string to this session's log file.
+    #     """
+    #     timestamp = strftime('%m/%d/%Y, %H:%M:%S', localtime())
+    #     with open(self.session_log, 'a') as fh:
+    #         fh.write(f'{timestamp} -- {event}\n')
 
     def record_workflow_run(self, record: WorkflowRunRecord or None):
         """
         Append a JSON describing inference data to this session's manifest
         """
-        self.log_event(f'Ran model {record.model_id} on {record.input_filepath} to infer {record.output_filepath}')
+        self.log_info(f'Ran model {record.model_id} on {record.input_filepath} to infer {record.output_filepath}')
         with open(self.manifest_json, 'w+') as fh:
             json.dump(record.dict(), fh)
 
@@ -114,7 +124,7 @@ class Session(object):
             'object': mi,
             'params': params
         }
-        self.log_event(f'Loaded model {key}')
+        self.log_info(f'Loaded model {key}')
         return key
 
     def describe_loaded_models(self) -> dict:
diff --git a/tests/test_session.py b/tests/test_session.py
index 9679aad6..92f12321 100644
--- a/tests/test_session.py
+++ b/tests/test_session.py
@@ -12,7 +12,7 @@ class TestGetSessionObject(unittest.TestCase):
         self.assertIs(sesh, Session(), 'Re-initializing Session class returned a new object')
 
         from os.path import exists
-        self.assertTrue(exists(sesh.session_log), 'Session did not create a log file in the correct place')
+        self.assertTrue(exists(sesh.logfile), 'Session did not create a log file in the correct place')
         self.assertTrue(exists(sesh.manifest_json), 'Session did not create a manifest JSON file in the correct place')
 
     def test_changing_session_root_creates_new_directory(self):
@@ -39,11 +39,22 @@ class TestGetSessionObject(unittest.TestCase):
 
     def test_restart_session(self):
         sesh = Session()
-        logfile1 = sesh.session_log
+        logfile1 = sesh.logfile
         sesh.restart()
-        logfile2 = sesh.session_log
+        logfile2 = sesh.logfile
         self.assertNotEqual(logfile1, logfile2, 'Restarting session does not generate new logfile')
 
+    def test_log_warning(self):
+        # this is passing on its own but failing in conjunction with other tests
+        sesh = Session()
+        msg = 'A test warning'
+        sesh.log_info(msg)
+
+        with open(sesh.logfile, 'r') as fh:
+            log = fh.read()
+
+        self.assertTrue(msg in log)
+
     def test_session_records_workflow(self):
         import json
         from model_server.base.workflows import WorkflowRunRecord
-- 
GitLab