From d32ef602ca62d5b0d63853ae912b109522d688d9 Mon Sep 17 00:00:00 2001
From: Jan David Mol <mol@astron.nl>
Date: Wed, 5 Jan 2022 11:06:41 +0100
Subject: [PATCH] L2SS-555: Split off common archiver/retriever functions, have
 retreiver use same get_db_config function

---
 .../tangostationcontrol/toolkit/archiver.py   | 75 +------------------
 .../toolkit/archiver_util.py                  | 61 +++++++++++++++
 .../tangostationcontrol/toolkit/retriever.py  | 55 +++++++-------
 3 files changed, 89 insertions(+), 102 deletions(-)
 create mode 100644 tangostationcontrol/tangostationcontrol/toolkit/archiver_util.py

diff --git a/tangostationcontrol/tangostationcontrol/toolkit/archiver.py b/tangostationcontrol/tangostationcontrol/toolkit/archiver.py
index d6aada7d5..d6049f86e 100644
--- a/tangostationcontrol/tangostationcontrol/toolkit/archiver.py
+++ b/tangostationcontrol/tangostationcontrol/toolkit/archiver.py
@@ -3,6 +3,7 @@
 import logging
 
 from tango import DeviceProxy, AttributeProxy, DevState, DevFailed
+from tangostationcontrol.toolkit.archiver_util import get_db_config, attribute_name_from_url, device_name_url, split_tango_name
 
 import time
 import json
@@ -10,66 +11,6 @@ import pkg_resources
 from functools import wraps
 
 logger = logging.getLogger()
-
-def attribute_name_from_url(attribute_name:str):
-    """
-    For some operations Tango attribute must be transformed from the form 'tango://db:port/domain/family/name/attribute'
-    to canonical 'domain/family/name/attribute'
-    """
-    if attribute_name.startswith('tango://'):
-        return '/'.join(attribute_name.split('/')[3:])
-
-    if len(attribute_name.split('/')) != 4:
-        raise ValueError(f"Expected attribute of format 'domain/family/name/attribute', got {attribute_name}")
-
-    return attribute_name
-
-def attribute_name_url(attribute_name:str, tango_host:str = 'databaseds:10000'):
-    """
-    For some operations Tango devices must be transformed from the form 'domain/family/name/attribute'
-    to 'tango://db:port/domain/family/name/attribute'
-    """
-    if attribute_name.startswith('tango://'):
-        return attribute_name
-
-    if len(attribute_name.split('/')) != 4:
-        raise ValueError(f"Expected attribute name of format 'domain/family/name/attribute', got {attribute_name}")
-
-    return f"tango://{tango_host}/{attribute_name}"
-
-def device_name_url(device_name:str, tango_host:str = 'databaseds:10000'):
-    """
-    For some operations Tango devices must be transformed from the form 'domain/family/name'
-    to 'tango://db:port/domain/family/name'
-    """
-    if device_name.startswith('tango://'):
-        return device_name
-
-    if len(device_name.split('/')) != 3:
-        raise ValueError(f"Expected device name of format 'domain/family/name', got {device_name}")
-
-    return f"tango://{tango_host}/{device_name}"
-
-def split_tango_name(tango_fqname:str, tango_type:str):
-    """
-    Helper function to split device or attribute Tango full qualified names
-    into its components
-    """
-    if tango_type.lower() == 'device':
-        try:
-            domain, family, member = tango_fqname.split('/')
-            return domain, family, member
-        except ValueError as e:
-            raise ValueError(f"Could not parse device name {tango_fqname}. Please provide FQDN, e.g. STAT/Device/1") from e
-    elif tango_type.lower() == 'attribute':
-        try:
-            domain, family, member, name = tango_fqname.split('/')
-            return domain, family, member, name
-        except ValueError as e:
-            raise ValueError(f"Could not parse attribute name {tango_fqname}. Please provide FQDN, e.g. STAT/Device/1/Attribute") from e
-    else:
-        raise ValueError(f"Invalid value: {tango_type}. Please provide 'device' or 'attribute'.")
-
  
 def warn_if_attribute_not_found():
     """
@@ -111,24 +52,12 @@ class Archiver():
         self.es_list = [es_name for es_name in self.get_subscribers(from_db=False)]
         self.cm.write_attribute('Context',context)    # Set default Context Archiving for all the subscribers
     
-    def get_db_config(self, device_name:str) -> dict:
-        """
-        Retrieve the DB credentials from the Tango properties of Configuration Manager or EventSubscribers
-        """
-        device = DeviceProxy(device_name)
-        # example LibConfiguration property value:
-        # ['connect_string= user=postgres password=password host=archiver-timescale port=5432 dbname=hdb', 'host=archiver-timescale', 'libname=libhdb++timescale.so', 'dbname=hdb', 'port=5432', 'user=postgres', 'password=password']
-        config_strs = device.get_property('LibConfiguration')['LibConfiguration']
-
-        config = dict(config_str.split("=",1) for config_str in config_strs)
-        return config
-    
     def get_hdbpp_libname(self, device_name:str):
         """
         Get the hdbpp library name used by the Configuration Manager or by the EventSubscribers
         Useful in the case of different DBMS architectures (e.g. MySQL, TimescaleDB)
         """
-        config = self.get_db_config(device_name)
+        config = get_db_config(device_name)
         return config["libname"]
     
     def get_subscribers(self, from_db:bool=False):
diff --git a/tangostationcontrol/tangostationcontrol/toolkit/archiver_util.py b/tangostationcontrol/tangostationcontrol/toolkit/archiver_util.py
new file mode 100644
index 000000000..fb44006a3
--- /dev/null
+++ b/tangostationcontrol/tangostationcontrol/toolkit/archiver_util.py
@@ -0,0 +1,61 @@
+"""
+   Utility functions for the Archiver functionality.
+"""
+
+def get_db_config(self, device_name:str) -> dict:
+    """
+    Retrieve the DB credentials from the Tango properties of Configuration Manager or EventSubscribers
+    """
+    device = DeviceProxy(device_name)
+    # example LibConfiguration property value:
+    # ['connect_string= user=postgres password=password host=archiver-timescale port=5432 dbname=hdb', 'host=archiver-timescale', 'libname=libhdb++timescale.so', 'dbname=hdb', 'port=5432', 'user=postgres', 'password=password']
+    config_strs = device.get_property('LibConfiguration')['LibConfiguration']
+
+    config = dict(config_str.split("=",1) for config_str in config_strs)
+    return config
+
+def attribute_name_from_url(attribute_name:str):
+    """
+    For some operations Tango attribute must be transformed from the form 'tango://db:port/domain/family/name/attribute'
+    to canonical 'domain/family/name/attribute'
+    """
+    if attribute_name.startswith('tango://'):
+        return '/'.join(attribute_name.split('/')[3:])
+
+    if len(attribute_name.split('/')) != 4:
+        raise ValueError(f"Expected attribute of format 'domain/family/name/attribute', got {attribute_name}")
+
+    return attribute_name
+
+def device_name_url(device_name:str, tango_host:str = 'databaseds:10000'):
+    """
+    For some operations Tango devices must be transformed from the form 'domain/family/name'
+    to 'tango://db:port/domain/family/name'
+    """
+    if device_name.startswith('tango://'):
+        return device_name
+
+    if len(device_name.split('/')) != 3:
+        raise ValueError(f"Expected device name of format 'domain/family/name', got {device_name}")
+
+    return f"tango://{tango_host}/{device_name}"
+
+def split_tango_name(tango_fqname:str, tango_type:str):
+    """
+    Helper function to split device or attribute Tango full qualified names
+    into its components
+    """
+    if tango_type.lower() == 'device':
+        try:
+            domain, family, member = tango_fqname.split('/')
+            return domain, family, member
+        except ValueError as e:
+            raise ValueError(f"Could not parse device name {tango_fqname}. Please provide FQDN, e.g. STAT/Device/1") from e
+    elif tango_type.lower() == 'attribute':
+        try:
+            domain, family, member, name = tango_fqname.split('/')
+            return domain, family, member, name
+        except ValueError as e:
+            raise ValueError(f"Could not parse attribute name {tango_fqname}. Please provide FQDN, e.g. STAT/Device/1/Attribute") from e
+    else:
+        raise ValueError(f"Invalid value: {tango_type}. Please provide 'device' or 'attribute'.")
diff --git a/tangostationcontrol/tangostationcontrol/toolkit/retriever.py b/tangostationcontrol/tangostationcontrol/toolkit/retriever.py
index 827164e22..4a3c0be20 100644
--- a/tangostationcontrol/tangostationcontrol/toolkit/retriever.py
+++ b/tangostationcontrol/tangostationcontrol/toolkit/retriever.py
@@ -1,7 +1,7 @@
 #! /usr/bin/env python3
 
-from tango import DeviceProxy
-from tangostationcontrol.toolkit.archiver import split_tango_name
+from tango import DeviceProxy, AttributeProxy
+from tangostationcontrol.toolkit.archiver_util import split_tango_name
 
 from abc import ABC, abstractmethod
 from datetime import datetime, timedelta
@@ -16,25 +16,18 @@ class Retriever(ABC):
     The Retriever abstract class implements retrieve operations on a given DBMS
     """
     
-    def get_db_credentials(self):
-        """
-        Retrieves the DB credentials from the Tango properties of Configuration Manager
-        """
-        cm = DeviceProxy(self.cm_name)
-        config_list = list(cm.get_property('LibConfiguration')['LibConfiguration']) # dictionary {'LibConfiguration': list of strings}
-        if 'connect_string=' in config_list[0]: config_list.pop(0)  # possibly remove connect string because it causes errors
-        host = str([s for s in config_list if "host" in s][0].split('=')[1])
-        dbname = str([s for s in config_list if "dbname" in s][0].split('=')[1])
-        port = str([s for s in config_list if "port" in s][0].split('=')[1])
-        user = str([s for s in config_list if "user" in s][0].split('=')[1])
-        pw = str([s for s in config_list if "password" in s][0].split('=')[1])
-        return host,dbname,port,user,pw
-
-    def create_session(self,libname:str,user:str,pw:str,host:str,port:str,dbname:str):
+    def create_session(self, creds):
         """
-        Returns a session to a DBMS using default credentials.
+        Returns a session to a DBMS using the given credentials.
         """
-        connection_string = f"{libname}://{user}:{pw}@{host}:{port}/{dbname}"
+        libname = creds["libname"]
+        user = creds["user"]
+        password = creds["password"]
+        host = creds["host"]
+        port = creds["port"]
+        dbname = creds["dbname"]
+
+        connection_string = f"{libname}://{user}:{password}@{host}:{port}/{dbname}"
         engine = create_engine(connection_string)
         Session = sessionmaker(bind=engine)
         return Session
@@ -134,13 +127,15 @@ class RetrieverMySQL(Retriever):
         """
         Returns a session to a MySQL DBMS using default credentials.
         """
-        host,dbname,port,user,pw = super().get_db_credentials()
+        creds = get_db_config(self.cm_name)
+
         # Set sqlalchemy library connection
-        if host=='archiver-maria-db':
-            libname = 'mysql+pymysql'         
+        if creds["host"] == 'archiver-maria-db':
+            creds["libname"] = 'mysql+pymysql'         
         else:
-            raise ValueError(f"Invalid hostname: {host}")
-        Session = super().create_session(libname,user,pw,host,port,dbname)
+            raise ValueError(f"Invalid hostname: {creds['host']}, we only support 'archiver-maria-db'")
+
+        Session = self.create_session(creds)
         return Session()
     
     def set_archiver_base(self):
@@ -228,13 +223,15 @@ class RetrieverTimescale(Retriever):
         """
         Returns a session to a MySQL DBMS using default credentials.
         """
-        host,dbname,port,user,pw = super().get_db_credentials()
+        creds = get_db_config(self.cm_name)
+
         # Set sqlalchemy library connection        
-        if host=='archiver-timescale':
-            libname = 'postgresql+psycopg2'
+        if creds["host"] == 'archiver-timescale':
+            creds["libname"] = 'postgresql+psycopg2'
         else:
-            raise ValueError(f"Invalid hostname: {host}")
-        Session = super().create_session(libname,user,pw,host,port,dbname)
+            raise ValueError(f"Invalid hostname: {creds['host']}, we only support 'archiver-timescale'")
+
+        Session = self.create_session(creds)
         return Session()
     
     def set_archiver_base(self):
-- 
GitLab