diff --git a/tangostationcontrol/setup.cfg b/tangostationcontrol/setup.cfg index ccbd782a1cfacbd44ebef9c3c57446b56dfd293d..99960c1ba63be6bbad187a84ced89b4d1c700206 100644 --- a/tangostationcontrol/setup.cfg +++ b/tangostationcontrol/setup.cfg @@ -17,7 +17,6 @@ classifier = Operating System :: POSIX :: Linux Programming Language :: Python Programming Language :: Python :: 3 - Programming Language :: Python :: 3.6 Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 Programming Language :: Python :: 3.9 @@ -27,7 +26,7 @@ classifier = package_dir= =./ packages=find: -python_requires = >=3.6 +python_requires = >=3.7 [options.packages.find] where=./ diff --git a/tangostationcontrol/tangostationcontrol/clients/opcua_client.py b/tangostationcontrol/tangostationcontrol/clients/opcua_client.py index 75d7da17007e12e06379fe9a96986f86310a5c3a..073933ad7dc01576e3817505c81c970fbe87ff1d 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 @@ -214,11 +216,10 @@ class OPCUAConnection(AsyncCommClient): node = await self.get_node(method_path[:-1]) result = await node.call_method(method_path[-1], *args) except Exception as e: - raise Exception(f"Calling method {method_path} failed") from e + raise RuntimeError(f"Calling method {method_path} failed") from e return result - def call_method(self, method_path, *args): return asyncio.run_coroutine_threadsafe(self._call_method(method_path, *args), self.event_loop).result() 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 adb4dd553a3c930207007e68b2ff0fa47ee154b6..d62a49e368598ccf5868f5c74402233d5eb403cf 100644 --- a/tangostationcontrol/tangostationcontrol/devices/beam.py +++ b/tangostationcontrol/tangostationcontrol/devices/beam.py @@ -156,10 +156,16 @@ class Beam(lofar_device): else: self.HBAT_beam_tracker.stop() - 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): @@ -172,10 +178,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) @@ -235,11 +247,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) @@ -250,10 +268,16 @@ class Beam(lofar_device): @DebugIt() @log_exceptions() @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/devices/sdp/statistics_collector.py b/tangostationcontrol/tangostationcontrol/devices/sdp/statistics_collector.py index 8c88a3a638eafdd152e0ee39be5db3dfc8e1063f..960fc49ab9d3fa3f9e3c2ad75429be901e420fbb 100644 --- a/tangostationcontrol/tangostationcontrol/devices/sdp/statistics_collector.py +++ b/tangostationcontrol/tangostationcontrol/devices/sdp/statistics_collector.py @@ -236,12 +236,15 @@ class XSTCollector(StatisticsCollector): self.parameters["xst_subbands"][subband_slot] = numpy.uint16(fields.subband_index) self.parameters["xst_integration_intervals"][subband_slot] = fields.integration_interval() - def xst_values(self, subband_indices=range(MAX_PARALLEL_SUBBANDS)): + def xst_values(self, subband_indices = None): """ xst_blocks, but as a matrix[len(subband_indices)][MAX_INPUTS][MAX_INPUTS] of complex values. The subband indices must be in [0..MAX_PARALLEL_SUBBANDS). By default, all recorded XSTs are returned. """ + if subband_indices is None: + subband_indices = range(self.MAX_PARALLEL_SUBBANDS) + matrix = numpy.zeros((len(subband_indices), self.MAX_INPUTS, self.MAX_INPUTS), dtype=numpy.complex64) xst_blocks = self.parameters["xst_blocks"] xst_conjugated = self.parameters["xst_conjugated"] 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..3ce57a0ae8660ac040a3cff0f5ef8680c5909ebc 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,9 @@ class TestClientServer(base.IntegrationAsyncTestCase): @asyncua.uamethod def throws(parent): - raise Exception("Expected test exception") + """This is not the exception raised by the opcua_client since it will + be caught""" + raise ArithmeticError("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 +64,9 @@ class TestClientServer(base.IntegrationAsyncTestCase): await self.server.stop() def fault_func(self): - raise Exception("FAULT") + """This is not the exception raised by the opcua_client since it will + be caught""" + raise ArithmeticError("FAULT") async def test_opcua_connection(self): await self.setup_server(14840) @@ -144,7 +148,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 +161,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 dcdd1940de9a697aefb4117d6de1edd7e31696a1..cb66e94706521dd867d75ce1c2b21173ff78a3e4 100644 --- a/tangostationcontrol/tangostationcontrol/toolkit/archiver.py +++ b/tangostationcontrol/tangostationcontrol/toolkit/archiver.py @@ -202,10 +202,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] @@ -236,14 +243,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: @@ -253,11 +268,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: diff --git a/tangostationcontrol/tox.ini b/tangostationcontrol/tox.ini index fecac58843e1806b1254750841d5b54d708193ca..97d91f8b083b7774a2b3a60cf8fa254572fa70f5 100644 --- a/tangostationcontrol/tox.ini +++ b/tangostationcontrol/tox.ini @@ -1,23 +1,29 @@ [tox] -minversion = 2.0 -envlist = py36,py37,py38,py39,py310,pep8 +minversion = 3.20 +envlist = py37,py38,py39,py310,pep8 skipsdist = True [testenv] usedevelop = True +; Module access is a bit of an ugly hack. This is due to testenv inheritance +; with sitepackages = True`, meaning that global packages can be accessed by the +; tox environment. Our tango images already have several dependencies system +; wide installed, however, the system wide installation will never look inside +; tox its virtualenv for packages. So accessing stestr and others fail.. We +; can't remove `sitepackages = True` either as we need access to tango and +; installing this package is non-trivial. The solution is to prevent calling +; binaries directly and utilizing python and tox variables to resolve the +; requested module. sitepackages = True -install_command = pip3 install {opts} {packages} +install_command = {envbindir}/pip3 install {opts} {packages} passenv = HOME setenv = VIRTUAL_ENV={envdir} PYTHONWARNINGS=default::DeprecationWarning - OS_STDOUT_CAPTURE=1 - OS_STDERR_CAPTURE=1 - OS_TEST_TIMEOUT=60 deps = -r{toxinidir}/../docker-compose/lofar-device-base/lofar-requirements.txt -r{toxinidir}/test-requirements.txt -commands = stestr run {posargs} +commands = {envpython} -m stestr run {posargs} [testenv:integration] ; Warning running integration tests will make changes to your docker system! @@ -25,39 +31,30 @@ commands = stestr run {posargs} passenv = TANGO_HOST setenv = TESTS_DIR=./tangostationcontrol/integration_test/{posargs} commands = - stestr run --serial + {envpython} -m stestr run --serial -; The access to coverage as module is a bit of an ugly hack. This is due to -; cover inheriting testenv which has `sitepackages = True`, meaning that global -; packages can be accessed by the tox environment. Our tango images already have -; coverage system wide installed, however, the system wide installation will -; never look inside tox its virtualenv for packages. So accessing stestr and -; others fail.. We can't remove `sitepackages = True` either as we need access -; to tango and installing this package is non-trivial. The solution is to -; prevent calling binaries directly and utilizing python to resolve the -; requested module. In addition stestr does not natively support generating -; coverage reports which is where the `PYTHON=python -m coverage run....` comes -; from. [testenv:cover] +; stestr does not natively support generating coverage reports use +; `PYTHON=python -m coverage run....` to overcome this. setenv = VIRTUAL_ENV={envdir} - PYTHON=python -m coverage run --source tangostationcontrol --parallel-mode + PYTHON={envpython} -m coverage run --source tangostationcontrol --parallel-mode deps = - -r{toxinidir}/test-requirements.txt -r{toxinidir}/../docker-compose/lofar-device-base/lofar-requirements.txt + -r{toxinidir}/test-requirements.txt commands = - python -m coverage erase - stestr run {posargs} - python -m coverage combine - python -m coverage html -d cover - python -m coverage xml -o coverage.xml - python -m coverage report + {envpython} -m coverage erase + {envpython} -m stestr run {posargs} + {envpython} -m coverage combine + {envpython} -m coverage html -d cover + {envpython} -m coverage xml -o coverage.xml + {envpython} -m coverage report ; TODO(Corne): Integrate Hacking to customize pep8 rules [testenv:pep8] commands = - doc8 docs/source/ --ignore D001 - flake8 + {envpython} -m doc8 docs/source/ --ignore D001 + {envpython} -m flake8 [testenv:bandit]; ; B104: hardcoded_bind_all_interfaces @@ -65,18 +62,18 @@ commands = ; It thus matters what interfaces Docker will bind our ; containers to, not what our containers listen on. commands = - bandit -r devices/ -n5 -ll -s B104 + {envpython} -m bandit -r devices/ -n5 -ll -s B104 [testenv:xenon]; commands = - xenon tangostationcontrol -b B -m A -a A + {envpython} -m xenon tangostationcontrol -b B -m A -a A [testenv:docs] deps = -r{toxinidir}/../docker-compose/lofar-device-base/lofar-requirements.txt -r{toxinidir}/docs/docs-requirements.txt commands = - sphinx-build -W -b html docs/source docs/build/html + sphinx-build -W -b html docs/source docs/build/html [flake8] filename = *.py,.stestr.conf,.txt