From cc22aca023297c676224176b97dc1e46be00891d Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 14:33:27 +0100
Subject: [PATCH 01/17] Replace existing cache code with diskcache calls

---
 lib/vsc/utils/cache.py | 99 +++++++++---------------------------------
 1 file changed, 21 insertions(+), 78 deletions(-)

diff --git a/lib/vsc/utils/cache.py b/lib/vsc/utils/cache.py
index 18588e6..a3c1527 100644
--- a/lib/vsc/utils/cache.py
+++ b/lib/vsc/utils/cache.py
@@ -28,17 +28,16 @@
 
 @author: Andy Georges (Ghent University)
 """
-import gzip
-import jsonpickle
-import os
+import logging
 import time
-import pickle
+import diskcache as dc
 
-from vsc.utils import fancylogger
 
 class FileCache(object):
     """File cache with a timestamp safety.
 
+    Wrapper around diskcache to retain the old API until all usage can be replaced.
+
     Stores data (something that can be pickled) into a dictionary
     indexed by the data.key(). The value stored is a tuple consisting
     of (the time of addition to the dictionary, the complete
@@ -65,52 +64,16 @@ def __init__(self, filename, retain_old=True, raise_unpickable=False):
 
         @param filename: (absolute) path to the cache file.
         """
+        del raise_unpickable
 
-        self.log = fancylogger.getLogger(self.__class__.__name__, fname=False)
-        self.filename = filename
-        self.retain_old = retain_old
+        self.retain_old = retain_old  # this is no longer used
 
-        self.new_shelf = {}
+        self.filename = filename
+        self.cache = dc.Cache(filename)
         if not retain_old:
-            self.log.info("Starting with a new empty cache, not retaining previous info if any.")
-            self.shelf = {}
-            return
-
-        try:
-            with open(self.filename, 'rb') as f:
-                try:
-                    g = gzip.GzipFile(mode='rb', fileobj=f)  # no context manager available in python 26 yet
-                    s = g.read()
-                except (IOError) as err:
-                    self.log.error("Cannot load data from cache file %s as gzipped json", self.filename)
-                    try:
-                        f.seek(0)
-                        self.shelf = pickle.load(f)
-                    except pickle.UnpicklingError as err:
-                        msg = "Problem loading pickle data from %s (corrupt data)" % (self.filename,)
-                        if raise_unpickable:
-                            self.log.raiseException(msg)
-                        else:
-                            self.log.error("%s. Continue with empty shelf: %s", msg, err)
-                            self.shelf = {}
-                    except (OSError, IOError):
-                        self.log.raiseException("Could not load pickle data from %s", self.filename)
-                else:
-                    try:
-                        self.shelf = jsonpickle.decode(s)
-                    except ValueError as err:
-                        self.log.error("Cannot decode JSON from %s [%s]", self.filename, err)
-                        self.log.info("Cache in %s starts with an empty shelf", self.filename)
-                        self.shelf = {}
-                finally:
-                    g.close()
-
-        except (OSError, IOError, ValueError, FileNotFoundError) as err:
-            self.log.warning("Could not access the file cache at %s [%s]", self.filename, err)
-            self.shelf = {}
-            self.log.info("Cache in %s starts with an empty shelf", (self.filename,))
-
-    def update(self, key, data, threshold):
+            self.cache.clear()
+
+    def update(self, key, data, threshold=None):
         """Update the given data if the existing data is older than the given threshold.
 
         @type key: something that can serve as a dictionary key (and thus can be pickled)
@@ -121,19 +84,11 @@ def update(self, key, data, threshold):
         @param data: whatever needs to be stored
         @param threshold: time in seconds
         """
+        old, old_timestamp = self.cache.get(key, default=(None, None))
         now = time.time()
-        old = self.load(key)
-        if old:
-            (ts, _) = old
-            if now - ts > threshold:
-                self.new_shelf[key] = (now, data)
-                return True
-            else:
-                self.new_shelf[key] = old
-                return False
-        else:
-            self.new_shelf[key] = (now, data)
-            return True
+
+        if not old or now - old_timestamp > threshold:
+            self.cache[key] = (data, now)
 
     def load(self, key):
         """Load the stored data for the given key along with the timestamp it was stored.
@@ -142,32 +97,20 @@ def load(self, key):
 
         @returns: (timestamp, data) if there is data for the given key, None otherwise.
         """
-        return self.new_shelf.get(key, None) or self.shelf.get(key, None)
+        return self.cache.get(key, default=None)
 
+    @DeprecationWarning
     def retain(self):
         """Retain non-updated data on close."""
         self.retain_old = True
 
+    @DeprecationWarning
     def discard(self):
         """Discard non-updated data on close."""
         self.retain_old = False
 
     def close(self):
         """Close the cache."""
-        dirname = os.path.dirname(self.filename)
-        if not os.path.exists(dirname):
-            os.makedirs(dirname)
-        with open(self.filename, 'wb') as fih:
-            if not fih:
-                self.log.error('cannot open the file cache at %s for writing', self.filename)
-            else:
-                if self.retain_old:
-                    self.shelf.update(self.new_shelf)
-                    self.new_shelf = self.shelf
-
-                with gzip.GzipFile(mode='wb', fileobj=fih) as zipf:
-                    pickled = jsonpickle.encode(self.new_shelf)
-                    # .encode() is required in Python 3, since we need to pass a bytestring
-                    zipf.write(pickled.encode())
-
-                self.log.info('closing the file cache at %s', self.filename)
+        self.cache.close()
+
+        logging.info('closing the file cache at %s', self.filename)

From 08c2c179bdf091ed5698ea4b0d980686b20384c4 Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 14:38:57 +0100
Subject: [PATCH 02/17] add diskcache as a dependency

---
 setup.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/setup.py b/setup.py
index 8a5d273..7c28a33 100755
--- a/setup.py
+++ b/setup.py
@@ -38,6 +38,7 @@
     'lockfile >= 0.9.1',
     'netifaces',
     'jsonpickle',
+    'diskcache',
 ]
 
 PACKAGE = {

From aacd3439761039323123088d579a0d3b6da0f4a0 Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 14:40:25 +0100
Subject: [PATCH 03/17] fix: headers

---
 setup.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/setup.py b/setup.py
index 7c28a33..10bdc50 100755
--- a/setup.py
+++ b/setup.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
-##
-# Copyright 2012-2020 Ghent University
+#
+# Copyright 2012-2023 Ghent University
 #
 # This file is part of vsc-utils,
 # originally created by the HPC team of Ghent University (http://ugent.be/hpc/en),
@@ -9,7 +9,7 @@
 # the Flemish Research Foundation (FWO) (http://www.fwo.be/en)
 # and the Department of Economy, Science and Innovation (EWI) (http://www.ewi-vlaanderen.be/en).
 #
-# http://github.com/hpcugent/vsc-utils
+# https://github.com/hpcugent/vsc-utils
 #
 # vsc-utils is free software: you can redistribute it and/or modify
 # it under the terms of the GNU Library General Public License as
@@ -23,7 +23,7 @@
 #
 # You should have received a copy of the GNU Library General Public License
 # along with vsc-utils. If not, see <http://www.gnu.org/licenses/>.
-##
+#
 """
 vsc-utils base distribution setup.py
 

From aceccfb55e90fb982ecdc438660b67257a9c529a Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 14:51:23 +0100
Subject: [PATCH 04/17] fix: catch corrupted cache file

---
 lib/vsc/utils/cache.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/vsc/utils/cache.py b/lib/vsc/utils/cache.py
index a3c1527..24290a6 100644
--- a/lib/vsc/utils/cache.py
+++ b/lib/vsc/utils/cache.py
@@ -28,9 +28,10 @@
 
 @author: Andy Georges (Ghent University)
 """
+import diskcache as dc
 import logging
+import shutil
 import time
-import diskcache as dc
 
 
 class FileCache(object):
@@ -69,7 +70,12 @@ def __init__(self, filename, retain_old=True, raise_unpickable=False):
         self.retain_old = retain_old  # this is no longer used
 
         self.filename = filename
-        self.cache = dc.Cache(filename)
+        try:
+            self.cache = dc.Cache(filename)
+        except:
+            shutil.rmtree(filename)
+            self.cache = dc.Cache(filename)
+
         if not retain_old:
             self.cache.clear()
 

From a21a63f3bb58a8164c40602d779b91784c432c67 Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 15:02:14 +0100
Subject: [PATCH 05/17] fix: revert catch

---
 lib/vsc/utils/cache.py | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lib/vsc/utils/cache.py b/lib/vsc/utils/cache.py
index 24290a6..b1dd897 100644
--- a/lib/vsc/utils/cache.py
+++ b/lib/vsc/utils/cache.py
@@ -30,7 +30,6 @@
 """
 import diskcache as dc
 import logging
-import shutil
 import time
 
 
@@ -70,11 +69,7 @@ def __init__(self, filename, retain_old=True, raise_unpickable=False):
         self.retain_old = retain_old  # this is no longer used
 
         self.filename = filename
-        try:
-            self.cache = dc.Cache(filename)
-        except:
-            shutil.rmtree(filename)
-            self.cache = dc.Cache(filename)
+        self.cache = dc.Cache(filename)
 
         if not retain_old:
             self.cache.clear()

From 8db2db06ff2e46908a810ca4e4276d7e7470c6aa Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 15:33:12 +0100
Subject: [PATCH 06/17] fix: remove cache tree when corrupt

---
 lib/vsc/utils/cache.py |  7 ++++++-
 test/cache.py          | 16 +++++++++-------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/lib/vsc/utils/cache.py b/lib/vsc/utils/cache.py
index b1dd897..24290a6 100644
--- a/lib/vsc/utils/cache.py
+++ b/lib/vsc/utils/cache.py
@@ -30,6 +30,7 @@
 """
 import diskcache as dc
 import logging
+import shutil
 import time
 
 
@@ -69,7 +70,11 @@ def __init__(self, filename, retain_old=True, raise_unpickable=False):
         self.retain_old = retain_old  # this is no longer used
 
         self.filename = filename
-        self.cache = dc.Cache(filename)
+        try:
+            self.cache = dc.Cache(filename)
+        except:
+            shutil.rmtree(filename)
+            self.cache = dc.Cache(filename)
 
         if not retain_old:
             self.cache.clear()
diff --git a/test/cache.py b/test/cache.py
index 5ac4e8f..03e48ad 100644
--- a/test/cache.py
+++ b/test/cache.py
@@ -102,15 +102,17 @@ def test_save_and_load(self):
 
         shutil.rmtree(tempdir)
 
-    def test_corrupt_gz_cache(self):
+    def test_corrupt_cache(self):
         """Test to see if we can handle a corrupt cache file"""
         tempdir = tempfile.mkdtemp()
-        # create a tempfilename
-        (handle, filename) = tempfile.mkstemp(dir=tempdir)
-        f = os.fdopen(handle, 'w')
-        f.write('blabla;not gz')
-        f.close()
-        FileCache(filename)
+
+        # create a bollocks cache file
+        with open(os.path.join(tempdir, "cache.db", "w")) as f:
+            f.write("blabla")
+
+        # this should clear the cache and create a new one
+        FileCache(tempdir)
+
         shutil.rmtree(tempdir)
 
     @mock.patch('vsc.utils.cache.jsonpickle.decode')

From e27a5f06144d33be929e3426b09109da3163d762 Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 15:35:13 +0100
Subject: [PATCH 07/17] fix: no  longer able to test for value decoding errors

---
 test/cache.py | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/test/cache.py b/test/cache.py
index 03e48ad..7d37007 100644
--- a/test/cache.py
+++ b/test/cache.py
@@ -114,22 +114,3 @@ def test_corrupt_cache(self):
         FileCache(tempdir)
 
         shutil.rmtree(tempdir)
-
-    @mock.patch('vsc.utils.cache.jsonpickle.decode')
-    def test_value_error(self, mock_decode):
-        "Test to see that a ValueError upon decoding gets caught correctly"
-        tempdir = tempfile.mkdtemp()
-        # create a tempfilename
-        (handle, filename) = tempfile.mkstemp(dir=tempdir)
-        f = os.fdopen(handle, 'wb')
-        g = gzip.GzipFile(mode='wb', fileobj=f)
-        g.write(b'blabla no json gzip stuffz')
-        g.close()
-
-        e = ValueError('unable to find valid JSON')
-        mock_decode.side_effect = e
-
-        fc = FileCache(filename)
-
-        self.assertTrue(fc.shelf == {})
-        shutil.rmtree(tempdir)

From 431bdf63165451aacac6e4ec5af4c06c68b4f3bb Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 15:46:11 +0100
Subject: [PATCH 08/17] fix: use logging instead of fancylogger

---
 lib/vsc/utils/nagios.py | 46 ++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/lib/vsc/utils/nagios.py b/lib/vsc/utils/nagios.py
index 7b7d0c5..8b7e02a 100644
--- a/lib/vsc/utils/nagios.py
+++ b/lib/vsc/utils/nagios.py
@@ -41,6 +41,7 @@
 @author: Luis Fernando Muñoz Mejías (Ghent University)
 """
 
+import logging
 import operator
 import os
 import pwd
@@ -50,12 +51,11 @@
 import time
 
 from vsc.utils.cache import FileCache
-from vsc.utils.fancylogger import getLogger
+from vsc.utils.py2vs3 import is_string, FileNotFoundErrorExc
 
-log = getLogger(__name__)
 
 NAGIOS_CACHE_DIR = '/var/cache'
-NAGIOS_CACHE_FILENAME_TEMPLATE = '%s.nagios.json.gz'
+NAGIOS_CACHE_FILENAME_TEMPLATE = '%s.nagios'
 
 NAGIOS_OK = 'OK'
 NAGIOS_WARNING = 'WARNING'
@@ -151,15 +151,13 @@ def __init__(self, nrange):
             @param nrange: nrange in [@][start:][end] format. If it is not a string, it is converted to
                           string and that string should allow conversion to float.
         """
-        self.log = getLogger(self.__class__.__name__, fname=False)
-
-        if not isinstance(nrange, str):
+        if not is_string(nrange):
             newnrange = str(nrange)
-            self.log.debug("nrange %s of type %s, converting to string (%s)", str(nrange), type(nrange), newnrange)
+            logging.debug("nrange %s of type %s, converting to string (%s)", str(nrange), type(nrange), newnrange)
             try:
                 float(newnrange)
             except ValueError:
-                self.log.raiseException("nrange %s (type %s) is not valid after conversion to string (newnrange %s)" %
+                logging.raiseException("nrange %s (type %s) is not valid after conversion to string (newnrange %s)" %
                                         (str(nrange), type(nrange), newnrange))
             nrange = newnrange
 
@@ -173,7 +171,7 @@ def parse(self, nrange):
         r = reg.search(nrange)
         if r:
             res = r.groupdict()
-            self.log.debug("parse: nrange %s gave %s", nrange, res)
+            logging.debug("parse: nrange %s gave %s", nrange, res)
 
             start_txt = res['start']
             if start_txt is None:
@@ -184,26 +182,26 @@ def parse(self, nrange):
                 try:
                     start = float(start_txt)
                 except ValueError:
-                    self.log.raiseException("Invalid start txt value %s" % start_txt)
+                    logging.raiseException("Invalid start txt value %s" % start_txt)
 
             end = res['end']
             if end is not None:
                 try:
                     end = float(end)
                 except ValueError:
-                    self.log.raiseException("Invalid end value %s" % end)
+                    logging.raiseException("Invalid end value %s" % end)
 
             neg = res['neg'] is not None
-            self.log.debug("parse: start %s end %s neg %s", start, end, neg)
+            logging.debug("parse: start %s end %s neg %s", start, end, neg)
         else:
-            self.log.raiseException('parse: invalid nrange %s.' % nrange)
+            logging.raiseException('parse: invalid nrange %s.' % nrange)
 
         def range_fn(test):
             # test inside nrange?
             try:
                 test = float(test)
             except ValueError:
-                self.log.raiseException("range_fn: can't convert test %s (type %s) to float" % (test, type(test)))
+                logging.raiseException("range_fn: can't convert test %s (type %s) to float" % (test, type(test)))
 
             start_res = True  # default: -inf < test
             if start is not None:
@@ -219,7 +217,7 @@ def range_fn(test):
             if neg:
                 tmp_res = operator.not_(tmp_res)
 
-            self.log.debug("range_fn: test %s start_res %s end_res %s result %s (neg %s)",
+            logging.debug("range_fn: test %s start_res %s end_res %s result %s (neg %s)",
                            test, start_res, end_res, tmp_res, neg)
             return tmp_res
 
@@ -261,7 +259,7 @@ def __init__(self, header, filename, threshold, nagios_username="nagios", world_
 
         self.nagios_username = nagios_username
 
-        self.log = getLogger(self.__class__.__name__, fname=False)
+        logging = getLogger(self.__class__.__name__, fname=False)
 
     def report_and_exit(self):
         """Unzips the cache file and reads the JSON data back in, prints the data and exits accordingly.
@@ -271,13 +269,13 @@ def report_and_exit(self):
         try:
             nagios_cache = FileCache(self.filename, True)
         except (IOError, OSError):
-            self.log.critical("Error opening file %s for reading", self.filename)
+            logging.critical("Error opening file %s for reading", self.filename)
             unknown_exit("%s nagios gzipped JSON file unavailable (%s)" % (self.header, self.filename))
 
         (timestamp, ((nagios_exit_code, nagios_exit_string), nagios_message)) = nagios_cache.load('nagios')
 
         if self.threshold <= 0 or time.time() - timestamp < self.threshold:
-            self.log.info("Nagios check cache file %s contents delivered: %s", self.filename, nagios_message)
+            logging.info("Nagios check cache file %s contents delivered: %s", self.filename, nagios_message)
             print("%s %s" % (nagios_exit_string, nagios_message))
             sys.exit(nagios_exit_code)
         else:
@@ -296,10 +294,11 @@ def cache(self, nagios_exit, nagios_message):
             nagios_cache = FileCache(self.filename)
             nagios_cache.update('nagios', (nagios_exit, nagios_message), 0)  # always update
             nagios_cache.close()
-            self.log.info("Wrote nagios check cache file %s at about %s", self.filename, time.ctime(time.time()))
+            logging.info("Wrote nagios check cache file %s at about %s", self.filename, time.ctime(time.time()))
         except (IOError, OSError):
             # raising an error is ok, since we usually do this as the very last thing in the script
-            self.log.raiseException("Cannot save to the nagios gzipped JSON file (%s)" % self.filename)
+            logging.error("Cannot save to the nagios gzipped JSON file (%s)" % self.filename)
+            raise Exception()
 
         try:
             p = pwd.getpwnam(self.nagios_username)
@@ -312,10 +311,11 @@ def cache(self, nagios_exit, nagios_message):
             if os.geteuid() == 0:
                 os.chown(self.filename, p.pw_uid, p.pw_gid)
             else:
-                self.log.warn("Not running as root: Cannot chown the nagios check file %s to %s",
+                logging.warning("Not running as root: Cannot chown the nagios check file %s to %s",
                               self.filename, self.nagios_username)
-        except (OSError, FileNotFoundError):
-            self.log.raiseException("Cannot chown the nagios check file %s to the nagios user" % (self.filename))
+        except (OSError, FileNotFoundErrorExc):
+            logging.error("Cannot chown the nagios check file %s to the nagios user" % (self.filename))
+            raise Exception()
 
         return True
 

From b6a7c6eff62c731855b72532c9ab8df3eca21426 Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 15:51:36 +0100
Subject: [PATCH 09/17] fix: logging

---
 lib/vsc/utils/nagios.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/vsc/utils/nagios.py b/lib/vsc/utils/nagios.py
index 8b7e02a..62c688b 100644
--- a/lib/vsc/utils/nagios.py
+++ b/lib/vsc/utils/nagios.py
@@ -87,7 +87,7 @@ def _real_exit(message, code, metrics=''):
         metrics = '|%s' % message[1]
     if len(msg) > NAGIOS_MAX_MESSAGE_LENGTH:
         # log long message but print truncated message
-        log.info("Nagios report %s: %s%s", exit_text, msg, metrics)
+        logging.info("Nagios report %s: %s%s", exit_text, msg, metrics)
         msg = msg[:NAGIOS_MAX_MESSAGE_LENGTH-3] + '...'
 
     print("%s %s%s" % (exit_text, msg, metrics))
@@ -259,8 +259,6 @@ def __init__(self, header, filename, threshold, nagios_username="nagios", world_
 
         self.nagios_username = nagios_username
 
-        logging = getLogger(self.__class__.__name__, fname=False)
-
     def report_and_exit(self):
         """Unzips the cache file and reads the JSON data back in, prints the data and exits accordingly.
 

From 389049b1b8b5cba8833ba38af514cec7574402dd Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Thu, 26 Jan 2023 15:55:33 +0100
Subject: [PATCH 10/17] parent b6a7c6eff62c731855b72532c9ab8df3eca21426 author
 Andy Georges <andy.georges@ugent.be> 1674744933 +0100 committer Andy Georges
 <andy.georges@ugent.be> 1677589584 +0100 gpgsig -----BEGIN PGP SIGNATURE-----

 iQIzBAABCAAdFiEEsRL1Gh5rbMUbS7S8e6TNo671utAFAmP9/FAACgkQe6TNo671
 utCySA//ZFeuoh5ZG3son8CXs0jZvfQBnxlnP0M3e4HbGi/KBINO95TB8TLjIbQ/
 ypvzEiHDeUIUsJn69ef645DGvNkop9H+1IepTh73WRNw/z13KBPjV/X7XoMUk4iZ
 7scmeHHbA6FH6KP4hgI5XCslGQKn6obucaCZgDE8e1bZldD8NuJxgQXapJvK0gQ/
 xJwarkZQ+4OTKxxRtCyWac33Ftx2z6mCbtdERae5KRuq+8oUOI1SG1q9kp+MGITa
 6L5GEED/7fsurfLEo0GRD9dHxlHd7RMO6GziCaIjcDeipFDP2bFM7x5uM0B9Nw2g
 dxO3gz7WG8YZF9vKtYuN1Sas1yGTLYBB5TjcI6lXhyCtQy2NVAeeAjbjCqr9dbzV
 n03oA9PdTojGq5dhXPwrkR17fXyoxiND7JI9ibdxL/yaGCZPJN0NgCHyZl5CnhOC
 o7qR3nhVZoJ9HalVzZnbjJcYjECWBYKYuRZoicclf56gvp6QvC/bDESQBBpe8SbF
 qIqc7SLQ+gddE8ajFRwiBeWU3UX9t/XPWpEvixvI4dXaZIpFydQGLsYQ3/9OehPv
 z0IzWp648H5hXfmYk3zpe25QfjGDj23OzwqHlNkr7KdReXck/2HhhDHL2Fw/h4jB
 CpYOlt05wLZlPnLt35sKomirx6yIOFU5rfkUdTU/0Ris90ZQWtI=
 =pjru
 -----END PGP SIGNATURE-----

test: fixes and refactoring to work with DiskCache
---
 lib/vsc/utils/cache.py        | 30 ++++++----------
 lib/vsc/utils/nagios.py       | 44 ++++++++++++++---------
 lib/vsc/utils/script_tools.py | 13 +++----
 test/cache.py                 | 31 +++++-----------
 test/nagios.py                | 67 ++++++++++++++++-------------------
 test/nagios_simple.py         | 38 ++++++++++++--------
 test/script_tools.py          |  7 ++--
 7 files changed, 108 insertions(+), 122 deletions(-)

diff --git a/lib/vsc/utils/cache.py b/lib/vsc/utils/cache.py
index 24290a6..05c7ef8 100644
--- a/lib/vsc/utils/cache.py
+++ b/lib/vsc/utils/cache.py
@@ -29,12 +29,10 @@
 @author: Andy Georges (Ghent University)
 """
 import diskcache as dc
-import logging
-import shutil
 import time
 
 
-class FileCache(object):
+class FileCache(dc.Cache):
     """File cache with a timestamp safety.
 
     Wrapper around diskcache to retain the old API until all usage can be replaced.
@@ -67,17 +65,12 @@ def __init__(self, filename, retain_old=True, raise_unpickable=False):
         """
         del raise_unpickable
 
-        self.retain_old = retain_old  # this is no longer used
+        super().__init__(filename)
 
-        self.filename = filename
-        try:
-            self.cache = dc.Cache(filename)
-        except:
-            shutil.rmtree(filename)
-            self.cache = dc.Cache(filename)
+        self.retain_old = retain_old  # this is no longer used
 
         if not retain_old:
-            self.cache.clear()
+            self.clear()
 
     def update(self, key, data, threshold=None):
         """Update the given data if the existing data is older than the given threshold.
@@ -90,11 +83,13 @@ def update(self, key, data, threshold=None):
         @param data: whatever needs to be stored
         @param threshold: time in seconds
         """
-        old, old_timestamp = self.cache.get(key, default=(None, None))
         now = time.time()
+        stored = self.set(key=key, value=(now, data), expire=threshold)
 
-        if not old or now - old_timestamp > threshold:
-            self.cache[key] = (data, now)
+        if stored:
+            return (now, data)
+        else:
+            return (None, None)
 
     def load(self, key):
         """Load the stored data for the given key along with the timestamp it was stored.
@@ -103,7 +98,7 @@ def load(self, key):
 
         @returns: (timestamp, data) if there is data for the given key, None otherwise.
         """
-        return self.cache.get(key, default=None)
+        return self.get(key, default=(None, None))
 
     @DeprecationWarning
     def retain(self):
@@ -115,8 +110,3 @@ def discard(self):
         """Discard non-updated data on close."""
         self.retain_old = False
 
-    def close(self):
-        """Close the cache."""
-        self.cache.close()
-
-        logging.info('closing the file cache at %s', self.filename)
diff --git a/lib/vsc/utils/nagios.py b/lib/vsc/utils/nagios.py
index 62c688b..16827b1 100644
--- a/lib/vsc/utils/nagios.py
+++ b/lib/vsc/utils/nagios.py
@@ -260,24 +260,26 @@ def __init__(self, header, filename, threshold, nagios_username="nagios", world_
         self.nagios_username = nagios_username
 
     def report_and_exit(self):
-        """Unzips the cache file and reads the JSON data back in, prints the data and exits accordingly.
+        """Reads the cache, prints the data and exits accordingly.
 
         If the cache data is too old (now - cache timestamp > self.threshold), a critical exit is produced.
         """
         try:
-            nagios_cache = FileCache(self.filename, True)
+            nagios_cache = FileCache(self.filename)
         except (IOError, OSError):
             logging.critical("Error opening file %s for reading", self.filename)
-            unknown_exit("%s nagios gzipped JSON file unavailable (%s)" % (self.header, self.filename))
+            unknown_exit("%s nagios cache unavailable (%s)" % (self.header, self.filename))
 
-        (timestamp, ((nagios_exit_code, nagios_exit_string), nagios_message)) = nagios_cache.load('nagios')
+        (_, nagios_exit_info) = nagios_cache.load('nagios')
+
+        if nagios_exit_info is None:
+            unknown_exit("%s nagios exit info expired" % self.header)
+
+        ((nagios_exit_code, nagios_exit_string), nagios_message) = nagios_exit_info
+
+        print("%s %s" % (nagios_exit_string, nagios_message))
+        sys.exit(nagios_exit_code)
 
-        if self.threshold <= 0 or time.time() - timestamp < self.threshold:
-            logging.info("Nagios check cache file %s contents delivered: %s", self.filename, nagios_message)
-            print("%s %s" % (nagios_exit_string, nagios_message))
-            sys.exit(nagios_exit_code)
-        else:
-            unknown_exit("%s gzipped JSON file too old (timestamp = %s)" % (self.header, time.ctime(timestamp)))
 
     def cache(self, nagios_exit, nagios_message):
         """Store the result in the cache file with a timestamp.
@@ -290,20 +292,28 @@ def cache(self, nagios_exit, nagios_message):
         """
         try:
             nagios_cache = FileCache(self.filename)
-            nagios_cache.update('nagios', (nagios_exit, nagios_message), 0)  # always update
+            nagios_cache.update('nagios', (nagios_exit, nagios_message), threshold=self.threshold)
             nagios_cache.close()
             logging.info("Wrote nagios check cache file %s at about %s", self.filename, time.ctime(time.time()))
         except (IOError, OSError):
             # raising an error is ok, since we usually do this as the very last thing in the script
-            logging.error("Cannot save to the nagios gzipped JSON file (%s)" % self.filename)
+            logging.error("Cannot save to the nagios cache (%s)", self.filename)
             raise Exception()
 
         try:
             p = pwd.getpwnam(self.nagios_username)
             if self.world_readable:
-                os.chmod(self.filename, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IWGRP | stat.S_IROTH)
+                os.chmod(
+                    self.filename,
+                    stat.S_IRUSR | stat.S_IWUSR |
+                    stat.S_IRGRP | stat.S_IWGRP | stat.S_IROTH |
+                    stat.S_IXUSR | stat.S_IXGRP
+                )
             else:
-                os.chmod(self.filename, stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IWGRP)
+                os.chmod(
+                    self.filename,
+                    stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IWGRP | stat.S_IXUSR | stat.S_IXGRP
+                )
 
             # only change owner/group when run as root
             if os.geteuid() == 0:
@@ -311,8 +321,8 @@ def cache(self, nagios_exit, nagios_message):
             else:
                 logging.warning("Not running as root: Cannot chown the nagios check file %s to %s",
                               self.filename, self.nagios_username)
-        except (OSError, FileNotFoundErrorExc):
-            logging.error("Cannot chown the nagios check file %s to the nagios user" % (self.filename))
+        except (OSError, FileNotFoundError):
+            logging.error("Cannot chown the nagios check file %s to the nagios user", self.filename)
             raise Exception()
 
         return True
@@ -439,7 +449,7 @@ def __init__(self, **kwargs):
         self._final = None
         self._final_state = None
 
-        self._threshold = 0
+        self._threshold = None
         self._report_and_exit = False
 
         self._world_readable = False
diff --git a/lib/vsc/utils/script_tools.py b/lib/vsc/utils/script_tools.py
index 6db30ce..5908180 100644
--- a/lib/vsc/utils/script_tools.py
+++ b/lib/vsc/utils/script_tools.py
@@ -37,7 +37,6 @@
 from copy import deepcopy
 
 import logging
-from vsc.utils import fancylogger
 from vsc.utils.availability import proceed_on_ha_service
 from vsc.utils.generaloption import SimpleOption
 from vsc.utils.lock import lock_or_bork, release_or_bork, LOCKFILE_DIR, LOCKFILE_FILENAME_TEMPLATE
@@ -138,8 +137,6 @@ def __init__(self, options, run_prologue=True, excepthook=None, **kwargs):
         else:
             sys.excepthook = excepthook
 
-        self.log = fancylogger.getLogger()
-
     def prologue(self):
         """Checks the options given for settings and takes appropriate action.
 
@@ -160,7 +157,7 @@ def prologue(self):
 
         # check for HA host
         if self.options.ha and not proceed_on_ha_service(self.options.ha):
-            self.log.warning("Not running on the target host %s in the HA setup. Stopping.", self.options.ha)
+            logging.warning("Not running on the target host %s in the HA setup. Stopping.", self.options.ha)
             self.nagios_reporter.ok("Not running on the HA master.")
             sys.exit(NAGIOS_EXIT_OK)
 
@@ -169,7 +166,7 @@ def prologue(self):
                                                    threshold=self.options.nagios_check_interval_threshold * 2)
             lock_or_bork(self.lockfile, self.nagios_reporter)
 
-        self.log.info("%s has started", _script_name(sys.argv[0]))
+        logging.info("%s has started", _script_name(sys.argv[0]))
 
     def _epilogue(self):
         if not self.options.disable_locking and not self.options.dry_run:
@@ -184,7 +181,7 @@ def epilogue(self, nagios_message, nagios_thresholds=None):
 
         nagios_thresholds['message'] = nagios_message
         self.nagios_reporter._eval_and_exit(**nagios_thresholds)
-        self.log.info("%s has finished", _script_name(sys.argv[0]))  # may not be reached
+        logging.info("%s has finished", _script_name(sys.argv[0]))  # may not be reached
 
     def ok(self, nagios_message):
         """Run at the end of a script and force an OK exit."""
@@ -221,8 +218,8 @@ def critical_exception_handler(self, tp, value, traceback):
 
         This function is meant to be used as sys.excepthook
         """
-        self.log.exception("unhandled exception detected: %s - %s", tp, value)
-        self.log.debug("traceback %s", traceback)
+        logging.exception("unhandled exception detected: %s - %s", tp, value)
+        logging.debug("traceback %s", traceback)
         message = "Script failure: %s - %s" % (tp, value)
         self.critical(message)
 
diff --git a/test/cache.py b/test/cache.py
index 7d37007..55dc6c8 100644
--- a/test/cache.py
+++ b/test/cache.py
@@ -38,6 +38,7 @@
 import sys
 import random
 
+from pathlib import PurePath
 from vsc.install.testing import TestCase
 from vsc.utils.cache import FileCache
 
@@ -61,9 +62,7 @@ def test_contents(self):
         data, threshold = get_rand_data()
 
         # create a tempfilename
-        (handle, filename) = tempfile.mkstemp(dir='/tmp')
-        os.unlink(filename)
-        os.close(handle)
+        filename = PurePath("/tmp") / next(tempfile._get_candidate_names())
         cache = FileCache(filename)
         for (key, value) in data.items():
             cache.update(key, value, threshold)
@@ -72,7 +71,7 @@ def test_contents(self):
         for key in data.keys():
             info = cache.load(key)
             self.assertFalse(info is None)
-            (ts, value) = info
+            ts, value = info
             self.assertTrue(value == data[key])
             self.assertTrue(ts <= now)
 
@@ -80,12 +79,10 @@ def test_save_and_load(self):
         """Check if the loaded data is the same as the saved data."""
         # test with random data
         data, threshold = get_rand_data()
-        tempdir = tempfile.mkdtemp()
-        # create a tempfilename
-        (handle, filename) = tempfile.mkstemp(dir=tempdir)
-        os.close(handle)
-        shutil.rmtree(tempdir)
+
+        filename = PurePath("/tmp") / next(tempfile._get_candidate_names())
         cache = FileCache(filename)
+
         for (key, value) in data.items():
             cache.update(key, value, threshold)
         cache.close()
@@ -93,24 +90,12 @@ def test_save_and_load(self):
         now = time.time()
         new_cache = FileCache(filename)
         for key in data.keys():
-            info = cache.load(key)
+            info = new_cache.load(key)
             self.assertTrue(info is not None)
             (ts, value) = info
             self.assertTrue(value == data[key])
             self.assertTrue(ts <= now)
         new_cache.close()
 
-        shutil.rmtree(tempdir)
-
-    def test_corrupt_cache(self):
-        """Test to see if we can handle a corrupt cache file"""
-        tempdir = tempfile.mkdtemp()
-
-        # create a bollocks cache file
-        with open(os.path.join(tempdir, "cache.db", "w")) as f:
-            f.write("blabla")
-
-        # this should clear the cache and create a new one
-        FileCache(tempdir)
+        shutil.rmtree(filename)
 
-        shutil.rmtree(tempdir)
diff --git a/test/nagios.py b/test/nagios.py
index 4ca9d77..c4d7cfc 100644
--- a/test/nagios.py
+++ b/test/nagios.py
@@ -28,15 +28,19 @@
 
 @author: Andy Georges (Ghent University)
 """
+import logging
 import os
 import tempfile
 import time
 import sys
 import random
+import shutil
 import string
+import tempfile
 from pwd import getpwuid
 from io import StringIO
 
+from pathlib import PurePath
 from vsc.install.testing import TestCase
 
 from vsc.utils.nagios import NagiosReporter, SimpleNagios
@@ -64,49 +68,48 @@ def test_eval(self):
 
     def test_cache(self):
         """Test the caching mechanism in the reporter."""
-        length = random.randint(1, 30)
-        exit_code = random.randint(0, 3)
-        threshold = random.randint(0, 10)
+        message = "huppeldepup a test string"
 
-        message = ''.join(random.choice(string.printable) for x in range(length))
-        message = message.rstrip()
+        threshold = None
+        filename = PurePath(next(tempfile._get_candidate_names()))
 
-        (handle, filename) = tempfile.mkstemp()
-        os.unlink(filename)
-        os.close(handle)
+        logging.info(f"Reporter using file {filename}")
         reporter = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
 
-        nagios_exit = [NAGIOS_EXIT_OK, NAGIOS_EXIT_WARNING, NAGIOS_EXIT_CRITICAL, NAGIOS_EXIT_UNKNOWN][exit_code]
-
-        reporter.cache(nagios_exit, message)
+        for exit_code in range(0, 4):
 
-        (handle, output_filename) = tempfile.mkstemp()
-        os.close(handle)
+            nagios_exit = [NAGIOS_EXIT_OK, NAGIOS_EXIT_WARNING, NAGIOS_EXIT_CRITICAL, NAGIOS_EXIT_UNKNOWN][exit_code]
+            logging.info("Caching the exit: %s", nagios_exit)
+            reporter.cache(nagios_exit, message)
 
-        try:
             old_stdout = sys.stdout
             buffer = StringIO()
             sys.stdout = buffer
-            reporter_test = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
-            reporter_test.report_and_exit()
-        except SystemExit as err:
-            line = buffer.getvalue().rstrip()
+
+            try:
+                reporter_test = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
+                reporter_test.report_and_exit()
+            except SystemExit as err:
+                line = buffer.getvalue().rstrip()
+                logging.info("Retrieved buffer value: %s", line)
+                logging.info("Retrieved exit code: %s", err.code)
+                logging.info("Expected exit value: %s", nagios_exit)
+                self.assertTrue(err.code == exit_code)
+                self.assertTrue(line == "%s %s" % (nagios_exit[1], message))
+
             sys.stdout = old_stdout
             buffer.close()
-            self.assertTrue(err.code == nagios_exit[0])
-            self.assertTrue(line == "%s %s" % (nagios_exit[1], message))
 
-        os.unlink(filename)
+        shutil.rmtree(filename)
 
     def test_threshold(self, message="Hello"):
         """Test the threshold borking mechanism in the reporter."""
         message = message.rstrip()
-        threshold = 1
+        threshold = 2
         if message == '':
             return
 
-        (handle, filename) = tempfile.mkstemp()
-        os.unlink(filename)
+        filename = PurePath(next(tempfile._get_candidate_names()))
         reporter = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
 
         # redirect stdout
@@ -116,11 +119,10 @@ def test_threshold(self, message="Hello"):
 
         nagios_exit = NAGIOS_EXIT_OK
         reporter.cache(nagios_exit, message)
-        os.close(handle)
 
         raised_exception = None
+        reporter_test = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
         try:
-            reporter_test = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
             reporter_test.report_and_exit()
         except SystemExit as err:
             raised_exception = err
@@ -128,19 +130,13 @@ def test_threshold(self, message="Hello"):
                          "Exit with status when the cached data is recent")
         # restore stdout
         buff.close()
-        sys.stdout = old_stdout
 
-        reporter = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
-        reporter.cache(nagios_exit, message)
         time.sleep(threshold + 1)
-        # redirect stdout
-        old_stdout = sys.stdout
         buff = StringIO()
         sys.stdout = buff
-
         raised_exception = None
+        reporter_test = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
         try:
-            reporter_test = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
             reporter_test.report_and_exit()
         except SystemExit as err:
             raised_exception = err
@@ -151,8 +147,7 @@ def test_threshold(self, message="Hello"):
         sys.stdout = old_stdout
         self.assertEqual(raised_exception.code, NAGIOS_EXIT_UNKNOWN[0],
                          "Too old caches lead to unknown status")
-        self.assertTrue(line.startswith("%s test_cache gzipped JSON file too old (timestamp =" %
-                                        (NAGIOS_EXIT_UNKNOWN[1])))
+        self.assertTrue(line.startswith("%s test_cache nagios exit info expired" % (NAGIOS_EXIT_UNKNOWN[1])))
 
-        os.unlink(filename)
+        shutil.rmtree(filename)
 
diff --git a/test/nagios_simple.py b/test/nagios_simple.py
index 0799371..e68a2b0 100644
--- a/test/nagios_simple.py
+++ b/test/nagios_simple.py
@@ -28,12 +28,15 @@
 
 @author: Stijn De Weirdt (Ghent University)
 """
+import logging
 import os
 import tempfile
+import shutil
 import sys
 import stat
 from pwd import getpwuid
 from io import StringIO
+from pathlib import PurePath
 
 from vsc.install.testing import TestCase
 
@@ -157,31 +160,36 @@ def test_simple_nagios_instance_and_nagios_exit(self):
 
     def test_cache(self):
         """Test the caching"""
-        (handle, filename) = tempfile.mkstemp()
-        os.unlink(filename)
+        message = "huppeldepup a test string"
+
+        threshold = None
+        filename = PurePath(next(tempfile._get_candidate_names()))
 
-        n = SimpleNagios(_cache=filename, _cache_user=self.nagios_user)
+        logging.info(f"Reporter using file {filename}")
+        simple_nagios = SimpleNagios(_cache=filename, _cache_user=self.nagios_user)
         message = "mywarning"
-        n.warning(message)
-        os.close(handle)
+        simple_nagios.warning(message)
 
-        self.buffo.seek(0)
-        self.buffo.truncate(0)
+        old_stdout = sys.stdout
+        buffer = StringIO()
+        sys.stdout = buffer
 
-        raised_exception = None
+        reporter_test = NagiosReporter('test_cache', filename, threshold, self.nagios_user)
         try:
-            reporter_test = NagiosReporter('test_cache', filename, -1, self.nagios_user)
             reporter_test.report_and_exit()
         except SystemExit as err:
-            raised_exception = err
-        bo = self.buffo.getvalue().rstrip()
+            line = buffer.getvalue().rstrip()
+            logging.info("Retrieved buffer value: %s", line)
+            logging.info("Retrieved exit code: %s", err.code)
+            logging.info("Expected exit value: %s", (NAGIOS_EXIT_WARNING[0], NAGIOS_EXIT_WARNING[1]))
+            self.assertTrue(err.code == 1)
+            self.assertTrue(line == "%s %s" % ("WARNING", message))
 
-        self.assertEqual(bo, "WARNING %s" % message)
-        self.assertEqual(raised_exception.code, NAGIOS_EXIT_WARNING[0])
+        sys.stdout = old_stdout
+        buffer.close()
 
-        statres = os.stat(filename)
+        shutil.rmtree(filename)
 
-        self.assertFalse(statres.st_mode & stat.S_IROTH)
 
     def test_world_readable(self):
         """Test world readable cache"""
diff --git a/test/script_tools.py b/test/script_tools.py
index bfe4012..950741b 100644
--- a/test/script_tools.py
+++ b/test/script_tools.py
@@ -30,6 +30,7 @@
 """
 import mock
 import logging
+import os
 import random
 import sys
 import tempfile
@@ -106,8 +107,8 @@ def do(self, dry_run):
 
 class MyCLI(CLI):
     TIMESTAMP_MANDATORY = False  # mainly for testing, you really should need this in production
-    TESTFILE = tempfile.mkstemp()[1]
-    TESTFILE2 = tempfile.mkstemp()[1]
+    TESTFILE = os.path.join("/tmp", next(tempfile._get_candidate_names()))
+    TESTFILE2 = os.path.join("/tmp", next(tempfile._get_candidate_names()))
 
     CLI_OPTIONS = {
         'magic': ('some magic', None, 'store', 'magicdef'),
@@ -138,7 +139,7 @@ def test_opts(self, prol):
             'ignoreconfigfiles': None,
             'info': False,
             'locking_filename': '/var/lock/setup.lock',
-            'nagios_check_filename': '/var/cache/setup.nagios.json.gz',
+            'nagios_check_filename': '/var/cache/setup.nagios',
             'nagios_check_interval_threshold': 0,
             'nagios_report': False,
             'nagios_user': 'nrpe',

From 5c98c91f72796e6cf880cc824d2b94589cc6b01f Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Tue, 28 Feb 2023 14:10:08 +0100
Subject: [PATCH 11/17] bump: version to 2.3.0

---
 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index 10bdc50..9de0e77 100755
--- a/setup.py
+++ b/setup.py
@@ -42,7 +42,7 @@
 ]
 
 PACKAGE = {
-    'version': '2.2.0',
+    'version': '2.3.0',
     'author': [ag, sdw],
     'maintainer': [ag, sdw],
     'excluded_pkgs_rpm': ['vsc', 'vsc.utils'],  # vsc is default, vsc.utils is provided by vsc-base

From 7168c08768dc0033fdeb07743ed5297699eb6b5a Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Tue, 28 Feb 2023 14:13:06 +0100
Subject: [PATCH 12/17] fix: remove py2/py3 import

---
 lib/vsc/utils/nagios.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/vsc/utils/nagios.py b/lib/vsc/utils/nagios.py
index 16827b1..1cc20bf 100644
--- a/lib/vsc/utils/nagios.py
+++ b/lib/vsc/utils/nagios.py
@@ -51,7 +51,6 @@
 import time
 
 from vsc.utils.cache import FileCache
-from vsc.utils.py2vs3 import is_string, FileNotFoundErrorExc
 
 
 NAGIOS_CACHE_DIR = '/var/cache'

From 2074256c839bf4f44c1a2404f2de028a1d3f8785 Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Tue, 28 Feb 2023 14:15:17 +0100
Subject: [PATCH 13/17] fix: replace is_string with isinstance

---
 lib/vsc/utils/nagios.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vsc/utils/nagios.py b/lib/vsc/utils/nagios.py
index 1cc20bf..187aac2 100644
--- a/lib/vsc/utils/nagios.py
+++ b/lib/vsc/utils/nagios.py
@@ -150,7 +150,7 @@ def __init__(self, nrange):
             @param nrange: nrange in [@][start:][end] format. If it is not a string, it is converted to
                           string and that string should allow conversion to float.
         """
-        if not is_string(nrange):
+        if not isinstance(nrange, str):
             newnrange = str(nrange)
             logging.debug("nrange %s of type %s, converting to string (%s)", str(nrange), type(nrange), newnrange)
             try:

From e57a0608b5fcd3359d2ade76c06c7385a1bcc613 Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Mon, 20 Mar 2023 10:31:00 +0100
Subject: [PATCH 14/17] fix: log exceptions and reraise

---
 lib/vsc/utils/nagios.py | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/vsc/utils/nagios.py b/lib/vsc/utils/nagios.py
index 187aac2..3f008eb 100644
--- a/lib/vsc/utils/nagios.py
+++ b/lib/vsc/utils/nagios.py
@@ -156,8 +156,9 @@ def __init__(self, nrange):
             try:
                 float(newnrange)
             except ValueError:
-                logging.raiseException("nrange %s (type %s) is not valid after conversion to string (newnrange %s)" %
+                logging.exception("nrange %s (type %s) is not valid after conversion to string (newnrange %s)" %
                                         (str(nrange), type(nrange), newnrange))
+                raise
             nrange = newnrange
 
         self.range_fn = self.parse(nrange)
@@ -181,26 +182,30 @@ def parse(self, nrange):
                 try:
                     start = float(start_txt)
                 except ValueError:
-                    logging.raiseException("Invalid start txt value %s" % start_txt)
+                    logging.exception("Invalid start txt value %s" % start_txt)
+                     raise
 
             end = res['end']
             if end is not None:
                 try:
                     end = float(end)
                 except ValueError:
-                    logging.raiseException("Invalid end value %s" % end)
+                    logging.exception("Invalid end value %s" % end)
+                    raise
 
             neg = res['neg'] is not None
             logging.debug("parse: start %s end %s neg %s", start, end, neg)
         else:
-            logging.raiseException('parse: invalid nrange %s.' % nrange)
+            logging.exception('parse: invalid nrange %s.' % nrange)
+            raise
 
         def range_fn(test):
             # test inside nrange?
             try:
                 test = float(test)
             except ValueError:
-                logging.raiseException("range_fn: can't convert test %s (type %s) to float" % (test, type(test)))
+                logging.exception("range_fn: can't convert test %s (type %s) to float" % (test, type(test)))
+                raise
 
             start_res = True  # default: -inf < test
             if start is not None:

From 749cec9bd8e5233b1d73bd03c2b7fad766ef30f9 Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Mon, 20 Mar 2023 10:31:54 +0100
Subject: [PATCH 15/17] fix: reraise instead of raising an empty exception

---
 lib/vsc/utils/nagios.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/vsc/utils/nagios.py b/lib/vsc/utils/nagios.py
index 3f008eb..6cce96c 100644
--- a/lib/vsc/utils/nagios.py
+++ b/lib/vsc/utils/nagios.py
@@ -302,7 +302,7 @@ def cache(self, nagios_exit, nagios_message):
         except (IOError, OSError):
             # raising an error is ok, since we usually do this as the very last thing in the script
             logging.error("Cannot save to the nagios cache (%s)", self.filename)
-            raise Exception()
+            raise
 
         try:
             p = pwd.getpwnam(self.nagios_username)
@@ -327,7 +327,7 @@ def cache(self, nagios_exit, nagios_message):
                               self.filename, self.nagios_username)
         except (OSError, FileNotFoundError):
             logging.error("Cannot chown the nagios check file %s to the nagios user", self.filename)
-            raise Exception()
+            raise
 
         return True
 

From f8157479f7720aa0df17220ee66cd70b4fa60242 Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Mon, 20 Mar 2023 10:47:23 +0100
Subject: [PATCH 16/17] fix: indentation

---
 lib/vsc/utils/nagios.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vsc/utils/nagios.py b/lib/vsc/utils/nagios.py
index 6cce96c..e5c58af 100644
--- a/lib/vsc/utils/nagios.py
+++ b/lib/vsc/utils/nagios.py
@@ -183,7 +183,7 @@ def parse(self, nrange):
                     start = float(start_txt)
                 except ValueError:
                     logging.exception("Invalid start txt value %s" % start_txt)
-                     raise
+                    raise
 
             end = res['end']
             if end is not None:

From 4f9a89eb09dc9d1fbdcbc9a82dbfd3061b9a11bf Mon Sep 17 00:00:00 2001
From: Andy Georges <andy.georges@ugent.be>
Date: Mon, 20 Mar 2023 10:54:45 +0100
Subject: [PATCH 17/17] fix: lazy string interpolation

---
 lib/vsc/utils/nagios.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/vsc/utils/nagios.py b/lib/vsc/utils/nagios.py
index e5c58af..b7fd594 100644
--- a/lib/vsc/utils/nagios.py
+++ b/lib/vsc/utils/nagios.py
@@ -156,8 +156,10 @@ def __init__(self, nrange):
             try:
                 float(newnrange)
             except ValueError:
-                logging.exception("nrange %s (type %s) is not valid after conversion to string (newnrange %s)" %
-                                        (str(nrange), type(nrange), newnrange))
+                logging.exception(
+                    "nrange %s (type %s) is not valid after conversion to string (newnrange %s)",
+                    str(nrange), type(nrange), newnrange
+                )
                 raise
             nrange = newnrange
 
@@ -182,7 +184,7 @@ def parse(self, nrange):
                 try:
                     start = float(start_txt)
                 except ValueError:
-                    logging.exception("Invalid start txt value %s" % start_txt)
+                    logging.exception("Invalid start txt value %s", start_txt)
                     raise
 
             end = res['end']
@@ -190,13 +192,13 @@ def parse(self, nrange):
                 try:
                     end = float(end)
                 except ValueError:
-                    logging.exception("Invalid end value %s" % end)
+                    logging.exception("Invalid end value %s", end)
                     raise
 
             neg = res['neg'] is not None
             logging.debug("parse: start %s end %s neg %s", start, end, neg)
         else:
-            logging.exception('parse: invalid nrange %s.' % nrange)
+            logging.exception('parse: invalid nrange %s.', nrange)
             raise
 
         def range_fn(test):
@@ -204,7 +206,7 @@ def range_fn(test):
             try:
                 test = float(test)
             except ValueError:
-                logging.exception("range_fn: can't convert test %s (type %s) to float" % (test, type(test)))
+                logging.exception("range_fn: can't convert test %s (type %s) to float", test, type(test))
                 raise
 
             start_res = True  # default: -inf < test