Merge pull request apel#373 from Will-Cross1/356-integrate-354-sugges…

…tions

Integrate apel#354 suggestions

apel/db/apeldb.py

@@ -48,7 +48,7 @@ def __new__(cls, backend, host, port, username, pwd, db):
         except ImportError:
             logger.info('Cannot import mysql backend')
 
-        if backend not in list(BACKENDS.keys()):
+        if backend not in BACKENDS:
             raise ApelDbException('Unknown backend: %s' % (backend))
 
         backend = BACKENDS[backend]

apel/db/loader/star_parser.py

@@ -118,9 +118,7 @@ def parseStarRecord(self, xml_storage_record):
 
         # Here we copy keys from functions.
         # We only want to change 'RecordId' to 'RecordIdentity'.
-        nodes = {}.fromkeys([f == 'RecordId' and
-                                'RecordIdentity' or f for f in [S for S in functions]])
-        # nodes = {}.fromkeys(functions.keys())
+        nodes = dict.fromkeys('RecordIdentity' if f == 'RecordId' else f for f in functions)
         data = {}
 
         for node in nodes:

apel/db/records/job.py

@@ -170,6 +170,11 @@ def _check_factor(self, sfu, sf):
 
         return (sfu, sf)
 
+    def _positive(self, field_name):
+        """Check that a field's value is positive when cast to int"""
+        value = self.get_field(field_name)
+        return value is not None and int(value) > 0
+
     def get_ur(self, withhold_dns=False):
         '''
         Returns the JobRecord in CAR format. See
@@ -259,26 +264,26 @@ def get_ur(self, withhold_dns=False):
         service_level.appendChild(doc.createTextNode(str(self.get_field('ServiceLevel'))))
         ur.appendChild(service_level)
 
-        if self.get_field('MemoryReal') is not None and int(self.get_field('MemoryReal')) > 0:
+        if self._positive('MemoryReal'):
             pmem = doc.createElement('urf:Memory')
             pmem.setAttribute('urf:type', 'Physical')
             pmem.setAttribute('urf:storageUnit', 'KB')
             pmem.appendChild(doc.createTextNode(str(self.get_field('MemoryReal'))))
             ur.appendChild(pmem)
 
-        if self.get_field('MemoryVirtual') is not None and int(self.get_field('MemoryVirtual')) > 0:
+        if self._positive('MemoryVirtual'):
             vmem = doc.createElement('urf:Memory')
             vmem.setAttribute('urf:type', 'Shared')
             vmem.setAttribute('urf:storageUnit', 'KB')
             vmem.appendChild(doc.createTextNode(str(self.get_field('MemoryVirtual'))))
             ur.appendChild(vmem)
 
-        if self.get_field('NodeCount') is not None and int(self.get_field('NodeCount')) > 0:
+        if self._positive('NodeCount'):
             ncount = doc.createElement('urf:NodeCount')
             ncount.appendChild(doc.createTextNode(str(self.get_field('NodeCount'))))
             ur.appendChild(ncount)
 
-        if self.get_field('Processors') is not None and int(self.get_field('Processors')) > 0:
+        if self._positive('Processors'):
             procs = doc.createElement('urf:Processors')
             procs.appendChild(doc.createTextNode(str(self.get_field('Processors'))))
             ur.appendChild(procs)

apel/db/records/record.py

@@ -307,16 +307,15 @@ def _check_fields(self):
 
         # Check that no extra fields are specified.
         # Is this inefficient?
-        for key in list(contents.keys()):
+        for key in contents:
             if key not in self._all_fields:
                 raise InvalidRecordException("Unexpected field " + key + " in message.")
 
         # Fill the dictionary even if we don't have the relevant data.
         # The string values are getting 'None' (not None!) instead of going into the
         # DB as NULL.
-        current_keys = list(contents.keys())
         for key in self._msg_fields:
-            if key not in current_keys: # key not already in the dictionary
+            if key not in contents:  # key not already in the dictionary
                 contents[key] = "None"
             if check_for_null(contents[key]):
                 contents[key] = "None"

apel/ldap/query.py

@@ -123,7 +123,7 @@ def fetch_specint(site, host='lcg-bdii.egi.eu', port=2170):
                 if fk.startswith(GLUE_CE_UNIQUE_ID):
                     name = fk.split('=')[1]
                     # do not overwrite values from first query
-                    if len([x for x in values if x[0] == name]) == 0:
+                    if not any(x[0] == name for x in values):
                         values.append((name, value))
 
     return values

apel/parsers/sge.py

@@ -89,9 +89,9 @@ def _load_multipliers(self):
         hosts = xml_str.getElementsByTagName("host")
 
         for host in hosts:
-            host_name = dict(host.attributes.items())["name"]
+            host_name = host.getAttribute("name")
             for resource in host.getElementsByTagName("resourcevalue"):
-                resource_name = dict(resource.attributes.items())["name"]
+                resource_name = resource.getAttribute("name")
                 if resource_name in ["cputmult", "wallmult"]:
                     try:
                         resource_value = float(resource.childNodes[0].data)

test/test_car_parser.py

@@ -178,7 +178,7 @@ def test_car_parser(self):
             self.assertIn('StartTime', cont)
             self.assertIn('EndTime', cont)
 
-            for key in list(cases[car].keys()):
+            for key in cases[car]:
                 if isinstance(cont[key], datetime.datetime):
                     if not datetimes_equal(cont[key], cases[car][key]):
                         self.fail("Datetimes don't match for key %s: %s, %s" % (key, cont[key], cases[car][key]))