L2SS-627: Fix previously unnoticed bugbear bugs

tangostationcontrol/tangostationcontrol/clients/opcua_client.py

@@ -184,7 +184,9 @@ class OPCUAConnection(AsyncCommClient):
             # NOTE: debug statement tries to get the qualified name, this may not always work. in that case forgo the name and just print the path
             node_name = str(node.get_browse_name())[len("QualifiedName(2:"):]
             logger.debug("connected OPC ua node {} of type {} to attribute with dimensions: {} x {} ".format(str(node_name)[:len(node_name)-1], str(ua_type)[len("VariantType."):], dim_x, dim_y))
-        except:
+        except Exception:
+            "B001 Do not use bare `except:`, it also catches unexpected events"
+            "like memory errors, interrupts, system exit"
             pass
 
         return prot_attr

tangostationcontrol/tangostationcontrol/common/lofar_logging.py

@@ -83,7 +83,7 @@ class LogAnnotator(logging.Formatter):
             This is derived by traversing the stack and find a Device as 'self'. In some cases,
             this fails, for example if a separate Thread is started for a certain Device. """
 
-        for frame,lineno in traceback.walk_stack(f=None):
+        for frame, _lineno in traceback.walk_stack(f=None):
             if "self" in frame.f_locals and isinstance(frame.f_locals["self"], Device):
                 return frame.f_locals["self"]
 

tangostationcontrol/tangostationcontrol/common/lofar_version.py

@@ -4,7 +4,9 @@ import functools
 import pkg_resources
 import re
 
-def get_repo(starting_directory: str = os.path.dirname(os.path.abspath(__file__)), limit = 10) -> git.Repo:
+basepath = os.path.dirname(os.path.abspath(__file__))
+
+def get_repo(starting_directory: str = basepath, limit = 10) -> git.Repo:
     """ Try finding the repository by traversing up the tree.
 
         By default, the repository containing this module is returned.
@@ -18,7 +20,7 @@ def get_repo(starting_directory: str = os.path.dirname(os.path.abspath(__file__)
         pass
 
     # We now have to traverse up the tree up until limit diretories
-    for i in range(limit):
+    for _i in range(limit):
         if directory == "/" or not os.path.exists(directory):
             break
 
@@ -95,7 +97,9 @@ def get_version(repo: git.Repo = None) -> str:
 # at least cache the current repo version immediately
 try:
     _ = get_version()
-except:
+except Exception:
+    "B001 Do not use bare `except:`, it also catches unexpected events like"
+    "memory errors, interrupts, system exit"
     pass
 
 

tangostationcontrol/tangostationcontrol/devices/beam.py

@@ -81,10 +81,16 @@ class Beam(lofar_device):
     # internal functions
     # --------
 
-    def _HBAT_delays(self, pointing_direction: numpy.array, timestamp: datetime.datetime = datetime.datetime.now()):
+    def _HBAT_delays(self, pointing_direction: numpy.array, timestamp: datetime.datetime = None):
         """
         Calculate the delays (in seconds) based on the pointing list and the timestamp
         """
+
+        if not timestamp:
+            "B008 Do not perform function calls in argument defaults.  The call"
+            "is performed only once at function definition time."
+            timestamp = datetime.datetime.now()
+
         delays = numpy.zeros((96,16), dtype=numpy.float64)
 
         for tile in range(96):
@@ -97,10 +103,16 @@ class Beam(lofar_device):
 
         return delays
 
-    def _HBAT_set_pointing(self, pointing_direction: numpy.array, timestamp: datetime.datetime = datetime.datetime.now()):
+    def _HBAT_set_pointing(self, pointing_direction: numpy.array, timestamp: datetime.datetime = None):
         """
         Uploads beam weights based on a given pointing direction 2D array (96 tiles x 3 parameters)
         """
+
+        if not timestamp:
+            "B008 Do not perform function calls in argument defaults.  The call"
+            "is performed only once at function definition time."
+            timestamp = datetime.datetime.now()
+
         # Retrieve delays from casacore
         delays = self._HBAT_delays(pointing_direction, timestamp)
         
@@ -159,11 +171,17 @@ class Beam(lofar_device):
     @DebugIt()
     @log_exceptions()
     @only_in_states([DevState.ON])
-    def HBAT_delays(self, pointing_direction: numpy.array, timestamp: datetime.datetime = datetime.datetime.now()):
+    def HBAT_delays(self, pointing_direction: numpy.array, timestamp: datetime.datetime = None):
         """
         Calculate the delays (in seconds) based on the pointing list and the timestamp
         TBD: antenna and reference positions will be retrieved from RECV and not stored as BEAM device properties
         """
+
+        if not timestamp:
+            "B008 Do not perform function calls in argument defaults.  The call"
+            "is performed only once at function definition time."
+            timestamp = datetime.datetime.now()
+
         pointing_direction = numpy.array(pointing_direction).reshape(96,3)
 
         delays = self._HBAT_delays(pointing_direction, timestamp)
@@ -173,10 +191,16 @@ class Beam(lofar_device):
     @command(dtype_in=DevVarStringArray)
     @DebugIt()
     @only_in_states([DevState.ON])
-    def HBAT_set_pointing(self, pointing_direction: list, timestamp: datetime.datetime = datetime.datetime.now()):
+    def HBAT_set_pointing(self, pointing_direction: list, timestamp: datetime.datetime = None):
         """
         Uploads beam weights based on a given pointing direction 2D array (96 tiles x 3 parameters)
         """
+
+        if not timestamp:
+            "B008 Do not perform function calls in argument defaults.  The call"
+            "is performed only once at function definition time."
+            timestamp = datetime.datetime.now()
+
         # Reshape the flatten input array
         pointing_direction = numpy.array(pointing_direction).reshape(96,3)
 

tangostationcontrol/tangostationcontrol/integration_test/default/client/test_opcua_client_against_server.py

@@ -45,7 +45,7 @@ class TestClientServer(base.IntegrationAsyncTestCase):
 
         @asyncua.uamethod
         def throws(parent):
-            raise Exception("Expected test exception")
+            raise RuntimeError("Expected test exception")
 
         multiply_method = await obj.add_method(idx, "multiply", multiply, [asyncua.ua.VariantType.Double, asyncua.ua.VariantType.Int64], [asyncua.ua.VariantType.Double])
         procedure_method = await obj.add_method(idx, "procedure", procedure, [], [])
@@ -62,7 +62,7 @@ class TestClientServer(base.IntegrationAsyncTestCase):
             await self.server.stop()
 
     def fault_func(self):
-        raise Exception("FAULT")
+        raise RuntimeError("FAULT")
 
     async def test_opcua_connection(self):
         await self.setup_server(14840)
@@ -144,7 +144,7 @@ class TestClientServer(base.IntegrationAsyncTestCase):
         try:
             await test_client.start()
 
-            with self.assertRaises(Exception):
+            with self.assertRaises(RuntimeError):
                 # correct signature is multiply(double,int64)
                 _ = await test_client._call_method(["multiply"], numpy.double(3.0), numpy.double(7))
         finally:
@@ -157,7 +157,7 @@ class TestClientServer(base.IntegrationAsyncTestCase):
         try:
             await test_client.start()
 
-            with self.assertRaises(Exception):
+            with self.assertRaises(RuntimeError):
                 await test_client._call_method(["throws"])
         finally:
             await test_client.stop()

tangostationcontrol/tangostationcontrol/statistics_writer/statistics_reader.py

@@ -100,7 +100,9 @@ class statistics_parser:
                     self.statistics.append(statistic)
                     self.statistics_dict[statistic.timestamp.isoformat(timespec="milliseconds")] = statistic
 
-                except:
+                except Exception:
+                    "B001 Do not use bare `except:`, it also catches unexpected"
+                    "events like memory errors, interrupts, system exit"
                     logger.warning(f"Encountered an error while parsing statistic. Skipped: {group_key}")
 
             logger.debug(f"Parsed {len(self.statistics)} statistics")

tangostationcontrol/tangostationcontrol/statistics_writer/udp_dev/udp_write_manager.py

@@ -24,7 +24,9 @@ class Statistics_Writer:
         # Create data directory if not exists
         try:
             os.makedirs('../data')
-        except:
+        except Exception:
+            "B001 Do not use bare `except:`, it also catches unexpected events"
+            "like memory errors, interrupts, system exit"
             print('Data directory already created')
 
         # create initial file

tangostationcontrol/tangostationcontrol/test/common/test_lofar_logging.py

@@ -100,9 +100,9 @@ class TestLofarLogging(base.TestCase):
         class Foo:
             @lofar_logging.log_exceptions()
             def exceptionalFunction(self):
-                raise Exception("test")
+                raise RuntimeError("test")
 
-        with self.assertRaises(Exception, msg="log_exceptions did not reraise exception"):
+        with self.assertRaises(RuntimeError, msg="log_exceptions did not reraise exception"):
             f = Foo()
             f.exceptionalFunction()
 

tangostationcontrol/tangostationcontrol/toolkit/archiver.py

@@ -177,10 +177,17 @@ class Archiver():
             else:
                 raise
 
-    def add_attributes_by_device(self, device_name, global_archive_period:int = None, es_name:str=None, exclude:list = []):
+    def add_attributes_by_device(self, device_name, global_archive_period:int = None, es_name:str=None, exclude:list = None):
         """
         Add sequentially all the attributes of the selected device in the event subscriber list, if not already present
         """
+        if not exclude:
+            """ B006 Do not use mutable data structures for argument defaults.
+            They are created during function definition time. All calls to the
+            function reuse this one instance of that data structure,
+            persisting changes between them"""
+            exclude = []
+
         d = DeviceProxy(device_fqdn(device_name))
         dev_attrs_list = d.get_attribute_list()
         exclude_list = [a.lower() for a in exclude]
@@ -211,14 +218,22 @@ class Archiver():
         self.cm.AttributeStop(attribute_name)
         self.cm.AttributeRemove(attribute_name)
         logger.warning(f"Attribute {attribute_name} removed!")
-    
-    def remove_attributes_by_device(self, device_name:str, exclude:list=[]):
+
+    def remove_attributes_by_device(self, device_name:str, exclude:list = None):
         """
         Stops the data archiving of all the attributes of the selected device, and remove them from the
         subscriber's list
         """
+
+        if not exclude:
+            """ B006 Do not use mutable data structures for argument defaults.
+            They are created during function definition time. All calls to the
+            function reuse this one instance of that data structure,
+            persisting changes between them"""
+            exclude = []
+
         d = DeviceProxy(device_fqdn(device_name))
-        dev_attrs_list = d.get_attribute_list()     # this list contains only the attribute names (not fqdn)
+        dev_attrs_list = d.get_attribute_list()
         exclude_list = [a.lower() for a in exclude]
         attrs_list = [a.lower() for a in list(dev_attrs_list) if a.lower() not in exclude_list]    # transform list for string comparison      
         for a in attrs_list:
@@ -228,11 +243,19 @@ class Archiver():
                     self.remove_attribute_from_archiver(attr_fullname)
             except Exception as e:
                 raise Exception from e
-    
-    def remove_attributes_in_error(self, exclude:list=[], es_name:str=None):
+
+    def remove_attributes_in_error(self, exclude:list = None, es_name:str=None):
         """
         Remove from the subscribers list all the attributes currently in error (not being archived)
         """
+
+        if not exclude:
+            """ B006 Do not use mutable data structures for argument defaults.
+            They are created during function definition time. All calls to the
+            function reuse this one instance of that data structure,
+            persisting changes between them"""
+            exclude = []
+
         if es_name is not None:
             es_list = [es_name]
         else: