Tangodict code improvements

jobQueue.py

@@ -53,11 +53,10 @@ def _getNextID(self):
 
         # If a job already exists in the queue at nextID, then try to find
         # an empty ID. If the queue is full, then return -1.
-        keys = self.liveJobs.keys()
-        if (str(id) in keys):
+        if id in self.liveJobs:
             id = -1
             for i in range(1, Config.MAX_JOBID + 1):
-                if (str(i) not in keys):
+                if i not in self.liveJobs:
                     id = i
                     break
 
@@ -82,7 +81,7 @@ def add(self, job):
             self.log.info("add|JobQueue is full")
             return -1
         self.log.debug("add|Gotten next ID: " + str(job.id))
-        self.log.info("add|Unassigning job ID: %d" % (job.id))
+        self.log.info("add|Unassigning job ID: %s" % (job.id))
         job.makeUnassigned()
         job.retries = 0
 
@@ -93,7 +92,7 @@ def add(self, job):
         self.log.debug("add| Acquired lock to job queue.")
 
         self.liveJobs.set(job.id, job)
-        job.appendTrace("%s|Added job %s:%d to queue" %
+        job.appendTrace("%s|Added job %s:%s to queue" %
                         (datetime.utcnow().ctime(), job.name, job.id))
 
         self.log.debug("Ref: " + str(job._remoteLocation))
@@ -103,7 +102,7 @@ def add(self, job):
         self.queueLock.release()
         self.log.debug("add|Releasing lock to job queue.")
 
-        self.log.info("Added job %s:%d to queue, details = %s" % 
+        self.log.info("Added job %s:%s to queue, details = %s" % 
             (job.name, job.id, str(job.__dict__)))
 
         return str(job.id)
@@ -137,7 +136,7 @@ def remove(self, id):
         self.log.debug("remove|Acquiring lock to job queue.")
         self.queueLock.acquire()
         self.log.debug("remove|Acquired lock to job queue.")
-        if str(id) in self.liveJobs.keys():
+        if id in self.liveJobs:
             self.liveJobs.delete(id)
             status = 0
 
@@ -163,7 +162,7 @@ def delJob(self, id, deadjob):
             status = -1
             self.queueLock.acquire()
             self.log.debug("delJob| Acquired lock to job queue.")
-            if str(id) in self.deadJobs.keys():
+            if id in self.deadJobs:
                 self.deadJobs.delete(id)
                 status = 0
             self.queueLock.release()
@@ -181,10 +180,7 @@ def get(self, id):
         """
         self.queueLock.acquire()
         self.log.debug("get| Acquired lock to job queue.")
-        if str(id) in self.liveJobs.keys():
-            job = self.liveJobs.get(id)
-        else:
-            job = None
+        job = self.liveJobs.get(id)
         self.queueLock.release()
         self.log.debug("get| Released lock to job queue.")
         return job
@@ -263,11 +259,11 @@ def makeDead(self, id, reason):
         self.queueLock.acquire()
         self.log.debug("makeDead| Acquired lock to job queue.")
         status = -1
-        if str(id) in self.liveJobs.keys():
-            self.log.info("makeDead| Found job ID: %d in the live queue" % (id))
+        if id in self.liveJobs:
+            self.log.info("makeDead| Found job ID: %s in the live queue" % (id))
             status = 0
             job = self.liveJobs.get(id)
-            self.log.info("Terminated job %s:%d: %s" %
+            self.log.info("Terminated job %s:%s: %s" %
                           (job.name, job.id, reason))
             self.deadJobs.set(id, job)           
             self.liveJobs.delete(id)

preallocator.py

@@ -31,7 +31,7 @@ def __init__(self, vmms):
     def poolSize(self, vmName):
         """ poolSize - returns the size of the vmName pool, for external callers
         """
-        if vmName not in self.machines.keys():
+        if vmName not in self.machines:
             return 0
         else:
             return len(self.machines.get(vmName)[0])
@@ -46,7 +46,7 @@ def update(self, vm, num):
         of machines as necessary.
         """
         self.lock.acquire()
-        if vm.name not in self.machines.keys():
+        if vm.name not in self.machines:
             self.machines.set(vm.name, [[], TangoQueue(vm.name)])
             self.log.debug("Creating empty pool of %s instances" % (vm.name))
         self.lock.release()
@@ -72,7 +72,7 @@ def allocVM(self, vmName):
         """ allocVM - Allocate a VM from the free list
         """
         vm = None
-        if vmName in self.machines.keys():
+        if vmName in self.machines:
             self.lock.acquire()
 
         if not self.machines.get(vmName)[1].empty():
@@ -203,7 +203,7 @@ def destroyVM(self, vmName, id):
         this function when the system is queiscent (pool size == free
         size)
         """
-        if vmName not in self.machines.keys():
+        if vmName not in self.machines:
             return -1
 
         dieVM = None
@@ -228,15 +228,15 @@ def destroyVM(self, vmName, id):
 
     def getAllPools(self):
         result = {}
-        for vmName in self.machines.keys():
+        for vmName in self.machines:
             result[vmName] = self.getPool(vmName)
         return result
 
     def getPool(self, vmName):
         """ getPool - returns the members of a pool and its free list
         """
         result = {}
-        if vmName not in self.machines.keys():
+        if vmName not in self.machines:
             return result
 
         result["total"] = []

tangoObjects.py

@@ -274,6 +274,9 @@ def __init__(self, object_name):
         self.r = getRedisConnection()
         self.hash_name = object_name
 
+    def __contains__(self, id):
+        return self.r.hexists(self.hash_name, str(id))
+
     def set(self, id, obj):
         pickled_obj = pickle.dumps(obj)
 
@@ -284,7 +287,7 @@ def set(self, id, obj):
         return str(id)
 
     def get(self, id):
-        if self.r.hexists(self.hash_name, str(id)):
+        if id in self:
             unpickled_obj = self.r.hget(self.hash_name, str(id))
             obj = pickle.loads(unpickled_obj)
             return obj
@@ -319,11 +322,14 @@ class TangoNativeDictionary(object):
     def __init__(self):
         self.dict = {}
 
+    def __contains__(self, id):
+        return str(id) in self.dict
+
     def set(self, id, obj):
         self.dict[str(id)] = obj
 
     def get(self, id):
-        if str(id) in self.dict:
+        if id in self:
             return self.dict[str(id)]
         else:
             return None

tests/testJobQueue.py

@@ -101,7 +101,7 @@ def test_getNextPendingJob(self):
         self.jobQueue.assignJob(self.jobId2)
         self.jobQueue.unassignJob(self.jobId1)
         exp_id = self.jobQueue.getNextPendingJob()
-        self.assertMultiLineEqual(exp_id, self.jobId1)
+        self.assertMultiLineEqual(str(exp_id), self.jobId1)
 
     def test_getNextPendingJobReuse(self):
         return False

tests/testObjects.py

@@ -0,0 +1,61 @@
+from __future__ import print_function
+from builtins import range
+from builtins import str
+import unittest
+import redis
+
+from tangoObjects import TangoDictionary, TangoJob
+from config import Config
+
+
+class TestObjects(unittest.TestCase):
+
+    def setUp(self):
+        if Config.USE_REDIS:
+            __db = redis.StrictRedis(
+                Config.REDIS_HOSTNAME, Config.REDIS_PORT, db=0)
+            __db.flushall()
+
+        self.test_entries = {
+            "key": "value",
+            0: "0_value",
+            123: 456,
+        }
+
+    def runDictionaryTests(self):
+        test_dict = TangoDictionary("test")
+        self.assertEqual(test_dict.keys(), [])
+        self.assertEqual(test_dict.values(), [])
+
+        for key in self.test_entries:
+            test_dict.set(key, self.test_entries[key])
+
+        for key in self.test_entries:
+            self.assertTrue(key in test_dict)
+            self.assertEqual(test_dict.get(key), self.test_entries[key])
+
+        for (key, val) in test_dict.items():
+            self.assertEqual(self.test_entries.get(key), val)
+
+        self.assertEqual(test_dict.keys(), [str(key) for key in self.test_entries.keys()])
+        self.assertEqual(test_dict.values(), list(self.test_entries.values()))
+        self.assertTrue("key_not_present" not in test_dict)
+        self.assertEqual(test_dict.get("key_not_present"), None)
+
+        test_dict.set("key", "new_value")
+        self.assertEqual(test_dict.get("key"), "new_value")
+
+        test_dict.delete("key")
+        self.assertTrue("key" not in test_dict)
+
+    def test_nativeDictionary(self):
+        Config.USE_REDIS = False
+        self.runDictionaryTests()
+
+    def test_remoteDictionary(self):
+        Config.USE_REDIS = True
+        self.runDictionaryTests()
+
+
+if __name__ == '__main__':
+    unittest.main()