diff --git a/tangostationcontrol/tangostationcontrol/clients/opcua_client.py b/tangostationcontrol/tangostationcontrol/clients/opcua_client.py index 75d7da17007e12e06379fe9a96986f86310a5c3a..813ec9908a55686a2359a22926e0beec9e07dd63 100644 --- a/tangostationcontrol/tangostationcontrol/clients/opcua_client.py +++ b/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 diff --git a/tangostationcontrol/tangostationcontrol/common/lofar_logging.py b/tangostationcontrol/tangostationcontrol/common/lofar_logging.py index f6c6457d2d002276b91e6115026f892b6cb22fdf..323c1afffd31653158c16561cf7c6e719211ddde 100644 --- a/tangostationcontrol/tangostationcontrol/common/lofar_logging.py +++ b/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"] diff --git a/tangostationcontrol/tangostationcontrol/common/lofar_version.py b/tangostationcontrol/tangostationcontrol/common/lofar_version.py index 89cb22f9fe6b76caf438984c673b21d00bc25645..be923cf9f1958555e4a397bfa5bffa33daaf3580 100644 --- a/tangostationcontrol/tangostationcontrol/common/lofar_version.py +++ b/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 diff --git a/tangostationcontrol/tangostationcontrol/devices/beam.py b/tangostationcontrol/tangostationcontrol/devices/beam.py index 886affe25ecb1d45f4b44e594d73f7b690c37d4b..ac6accea05a7131ff58f0c48daf880267fdb098d 100644 --- a/tangostationcontrol/tangostationcontrol/devices/beam.py +++ b/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) diff --git a/tangostationcontrol/tangostationcontrol/integration_test/default/client/test_opcua_client_against_server.py b/tangostationcontrol/tangostationcontrol/integration_test/default/client/test_opcua_client_against_server.py index bdd4e43b94b1a98596f339e220ee31116818fc37..8a1d035f64a57b9b96046c0e27720623fa62b9ec 100644 --- a/tangostationcontrol/tangostationcontrol/integration_test/default/client/test_opcua_client_against_server.py +++ b/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() diff --git a/tangostationcontrol/tangostationcontrol/statistics_writer/statistics_reader.py b/tangostationcontrol/tangostationcontrol/statistics_writer/statistics_reader.py index eaaa2d13ef1c139e7b924c474810ae2e18b29bf0..35bd00cb56e028a533cadb7d5c16c20451ecce0b 100644 --- a/tangostationcontrol/tangostationcontrol/statistics_writer/statistics_reader.py +++ b/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") diff --git a/tangostationcontrol/tangostationcontrol/statistics_writer/udp_dev/udp_write_manager.py b/tangostationcontrol/tangostationcontrol/statistics_writer/udp_dev/udp_write_manager.py index 0c11f6a82dc11f8151eb771b90033feb38ef9c42..657607d688425ea862aeaf8c08238639acc98873 100644 --- a/tangostationcontrol/tangostationcontrol/statistics_writer/udp_dev/udp_write_manager.py +++ b/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 diff --git a/tangostationcontrol/tangostationcontrol/test/common/test_lofar_logging.py b/tangostationcontrol/tangostationcontrol/test/common/test_lofar_logging.py index 5140d05e58ca370509a080211ca96f2df21fe399..4d59590c8fbe0b5a9d166c65b3abde022492e606 100644 --- a/tangostationcontrol/tangostationcontrol/test/common/test_lofar_logging.py +++ b/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() diff --git a/tangostationcontrol/tangostationcontrol/toolkit/archiver.py b/tangostationcontrol/tangostationcontrol/toolkit/archiver.py index eae77f0abee704eaf406d4456fd0c0e4e4297d2d..595c2a52508bad14f1591646ee73efc75b743629 100644 --- a/tangostationcontrol/tangostationcontrol/toolkit/archiver.py +++ b/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: