From ebe9fff8672ff0366adef6fb7124b085e8befc06 Mon Sep 17 00:00:00 2001 From: Gianluca Marotta Date: Tue, 15 Dec 2020 17:20:00 +0100 Subject: [PATCH 1/6] CT-205 Added transaction_id decorator --- .../csp_lmc_common/utils/decorators.py | 83 ++++++++----------- 1 file changed, 36 insertions(+), 47 deletions(-) diff --git a/csp-lmc-common/csp_lmc_common/utils/decorators.py b/csp-lmc-common/csp_lmc_common/utils/decorators.py index 9b86368..38d2664 100644 --- a/csp-lmc-common/csp_lmc_common/utils/decorators.py +++ b/csp-lmc-common/csp_lmc_common/utils/decorators.py @@ -1,11 +1,44 @@ import sys import os - import tango -from .cspcommons import CmdExecState -from ska.base.control_model import AdminMode,ObsState import functools +import json +from ska.log_transactions import transaction +from ska.base.control_model import AdminMode,ObsState +from .cspcommons import CmdExecState + +def transaction_id(func): + """ + This decorator add a transaction id to the input of the decorated method. + The input of the decorated method is a json string. + + """ + @functools.wraps(func) + def wrap(*args, **kwargs): + + # Argument of the decorated method can be either positional or named. + # Following code manage both cases. + # A named argument occurs when applied to "do" method of SKA Base Command Classes. + # argument must be single. + # note that args[0] is the "self" object of the class of the decorated method. + if kwargs: + keys=list(kwargs.keys()) + argin = kwargs[keys[0]] + elif len(args)>1: + argin = args[1] + else: + raise Exception('No argument are passed to the transaction ID decorator') + parameters =json.loads(argin) + obj = args[0] + #note: obj.name is the Command Class Name. + with transaction(obj.name, parameters,logger=obj.logger) as transaction_id: + + parameters['transaction_id'] = transaction_id + + argin = json.dumps(parameters) + return func(obj,argin) + return wrap class AdminModeCheck(object): """ @@ -47,50 +80,6 @@ class AdminModeCheck(object): return f(*args, **kwargs) return admin_mode_check - -VALID_OBS_STATE = {'reset': [ObsState.ABORTED], - 'configscan': [ObsState.IDLE, ObsState.READY], - 'scan': [ObsState.READY], - 'endscan': [ObsState.SCANNING], - 'gotoidle': [ObsState.READY, ObsState.IDLE], - 'addresources': [ObsState.IDLE], - 'removeresources': [ObsState.IDLE] - } - - -class ObsStateCheck(object): - """ - Class designed to be a decorator for the CspMaster methods. - It checks the obsMode attribute value - """ - - def __init__(self, args=False, kwargs=False): - self._args = args - self._kwargs = kwargs - - def __call__(self, f): - @functools.wraps(f) - def obs_state_check(*args, **kwargs): - # get the device instance - dev_instance = args[0] - cmd_type = self._args - - dev_instance.logger.info("device obs_state: {}".format(dev_instance._obs_state)) - # Check the obsState attribute value: valid values for the command to - # execute are defined by VALID_OBS_STATE dictionary - if dev_instance._obs_state not in VALID_OBS_STATE[cmd_type]: - msg_args = (f.__name__, ObsState(dev_instance._obs_state).name) - err_msg = ("{} command can't be issued when the" - " obsState is {} ".format(*msg_args)) - - tango.Except.throw_exception("Command not executable", - err_msg, - "ObsStateCheck decorator", - tango.ErrSeverity.ERR) - return f(*args, **kwargs) - return obs_state_check - - class CmdInputArgsCheck(object): """ Class designed to be a decorator for the CspMaster methods. -- GitLab From 8db4023931ff9161e2908983923daeeb374c1bd5 Mon Sep 17 00:00:00 2001 From: Gianluca Marotta Date: Tue, 15 Dec 2020 18:30:01 +0100 Subject: [PATCH 2/6] CT-205 First implementation of unit test for transaction id decorator --- csp-lmc-common/requirements.txt | 4 ++- csp-lmc-common/setup.py | 2 +- .../tests/unit/transaction_id_test.py | 26 +++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 csp-lmc-common/tests/unit/transaction_id_test.py diff --git a/csp-lmc-common/requirements.txt b/csp-lmc-common/requirements.txt index f576b4d..a0e04f8 100644 --- a/csp-lmc-common/requirements.txt +++ b/csp-lmc-common/requirements.txt @@ -1,4 +1,6 @@ numpy == 1.17.2 pytango >= 9.3.2 jsonschema >= 3.2.0 -lmcbaseclasses >= 0.6.5 +lmcbaseclasses >= 0.7.2 +ska-skuid +ska-log-transactions diff --git a/csp-lmc-common/setup.py b/csp-lmc-common/setup.py index ea37628..65e4099 100644 --- a/csp-lmc-common/setup.py +++ b/csp-lmc-common/setup.py @@ -39,7 +39,7 @@ setup( install_requires = [ 'pytango >9.3.1', 'future', - 'lmcbaseclasses>=0.6.5' + 'lmcbaseclasses', ], dependency_links=[ 'https://nexus.engageska-portugal.pt/repository/pypi/simple' diff --git a/csp-lmc-common/tests/unit/transaction_id_test.py b/csp-lmc-common/tests/unit/transaction_id_test.py new file mode 100644 index 0000000..7441fed --- /dev/null +++ b/csp-lmc-common/tests/unit/transaction_id_test.py @@ -0,0 +1,26 @@ +from csp_lmc_common.utils.decorators import transaction_id +import json + +def test_transaction_id(): + class class_test: + def __init__(self): + self.name = 'name_test' + self.logger = None + @transaction_id + def test_func(self, argin): + return argin + + argin = '{"id":"test_id"}' + c = class_test() + + out = c.test_func(argin) + assert 'transaction_id' in out + + out = c.test_func(argin=argin) + assert 'transaction_id' in out + + argin = '{"transaction_id":"test_id"}' + out = c.test_func(argin) + out = json.loads(out) + assert out['transaction_id'] == 'test_id' + -- GitLab From 5d0e219975637c2b0dfb118374cd5350bbec7556 Mon Sep 17 00:00:00 2001 From: Gianluca Marotta Date: Thu, 17 Dec 2020 14:04:29 +0100 Subject: [PATCH 3/6] CT-205 Final implementation of unit test for transaction id decorator; removed unusued decorators in CSPSubarray code --- csp-lmc-common/csp_lmc_common/CspSubarray.py | 1 - .../csp_lmc_common/utils/decorators.py | 6 +-- .../tests/unit/transaction_id_test.py | 50 ++++++++++++++----- 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/csp-lmc-common/csp_lmc_common/CspSubarray.py b/csp-lmc-common/csp_lmc_common/CspSubarray.py index f52a859..2089ee1 100644 --- a/csp-lmc-common/csp_lmc_common/CspSubarray.py +++ b/csp-lmc-common/csp_lmc_common/CspSubarray.py @@ -40,7 +40,6 @@ from ska.base import SKASubarray, SKASubarrayStateModel from ska.base.commands import ActionCommand, ResultCode from ska.base.faults import CapabilityValidationError from ska.base.control_model import HealthState, AdminMode, ObsState, ObsMode -from .utils.decorators import AdminModeCheck, ObsStateCheck, SubarrayRejectCmd from .utils.cspcommons import CmdExecState from . import release # PROTECTED REGION END # // CspSubarray.additionnal_import diff --git a/csp-lmc-common/csp_lmc_common/utils/decorators.py b/csp-lmc-common/csp_lmc_common/utils/decorators.py index 38d2664..7290b9a 100644 --- a/csp-lmc-common/csp_lmc_common/utils/decorators.py +++ b/csp-lmc-common/csp_lmc_common/utils/decorators.py @@ -9,16 +9,16 @@ from .cspcommons import CmdExecState def transaction_id(func): """ - This decorator add a transaction id to the input of the decorated method. + Add a transaction id to the input of the decorated method. The input of the decorated method is a json string. """ @functools.wraps(func) def wrap(*args, **kwargs): - # Argument of the decorated method can be either positional or named. + # Argument of the decorated method can be either positional or keyword. # Following code manage both cases. - # A named argument occurs when applied to "do" method of SKA Base Command Classes. + # A keyword argument occurs when applied to "do" method of SKA Base Command Classes. # argument must be single. # note that args[0] is the "self" object of the class of the decorated method. if kwargs: diff --git a/csp-lmc-common/tests/unit/transaction_id_test.py b/csp-lmc-common/tests/unit/transaction_id_test.py index 7441fed..3857365 100644 --- a/csp-lmc-common/tests/unit/transaction_id_test.py +++ b/csp-lmc-common/tests/unit/transaction_id_test.py @@ -1,26 +1,52 @@ from csp_lmc_common.utils.decorators import transaction_id import json -def test_transaction_id(): - class class_test: - def __init__(self): - self.name = 'name_test' - self.logger = None - @transaction_id - def test_func(self, argin): - return argin +class class_test: + """ A test class for the transaction id. + It has the few essential attributes + and the decorated method called in tests. + """ + def __init__(self): + self.name = 'name_test' + self.logger = None + @transaction_id + def test_func(self, argin): + """ A method to be tested with the transaction id decorator. + it returns the input "argin" modified by the decorator + """ + return argin + +def test_transaction_id_with_positional_arguments(): + """ A test that shows that the transaction id is added to the argin + when a positional arguments is passed to the decorated function + """ argin = '{"id":"test_id"}' c = class_test() + argin = c.test_func(argin) + assert 'transaction_id' in argin - out = c.test_func(argin) - assert 'transaction_id' in out +def test_transaction_id_with_keyword_arguments(): + """ The transaction id is added to the argin + when a keyword argument is passed to the decorated function. + The key of the argument is irrelevant + """ + argin = '{"id":"test_id"}' + c = class_test() + argin = c.test_func(argin1=argin) + assert 'transaction_id' in argin - out = c.test_func(argin=argin) - assert 'transaction_id' in out + argin = c.test_func(argin2=argin) + assert 'transaction_id' in argin +def test_transaction_id_if_present(): + """ If the transaction id is already present in argin + it is not changed by the decorator + """ argin = '{"transaction_id":"test_id"}' + c = class_test() out = c.test_func(argin) out = json.loads(out) assert out['transaction_id'] == 'test_id' + -- GitLab From 1233cb9e93469f9884f1759eabc841ac70bb7ba4 Mon Sep 17 00:00:00 2001 From: Gianluca Marotta Date: Mon, 21 Dec 2020 19:47:16 +0100 Subject: [PATCH 4/6] CT-206 Implemented Transaction ID in CSP Subarray ConfigurationScan and AssignResources methods --- csp-lmc-common/csp_lmc_common/CspSubarray.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/csp-lmc-common/csp_lmc_common/CspSubarray.py b/csp-lmc-common/csp_lmc_common/CspSubarray.py index 2089ee1..811da7e 100644 --- a/csp-lmc-common/csp_lmc_common/CspSubarray.py +++ b/csp-lmc-common/csp_lmc_common/CspSubarray.py @@ -41,6 +41,7 @@ from ska.base.commands import ActionCommand, ResultCode from ska.base.faults import CapabilityValidationError from ska.base.control_model import HealthState, AdminMode, ObsState, ObsMode from .utils.cspcommons import CmdExecState +from .utils.decorators import transaction_id from . import release # PROTECTED REGION END # // CspSubarray.additionnal_import @@ -369,9 +370,17 @@ class CspSubarray(SKASubarray): message = "Off command completed OK" self.logger.info(message) return (ResultCode.OK, message) + + class AssignResourcesCommand(SKASubarray.AssignResourcesCommand): + + @transaction_id + def do(self,argin): + return super().do(argin) + self.logger.warning("Assign Resource Command not yet implemented in CSP Subarray. This is an instance of the lmcbaseclasses") class ConfigureCommand(SKASubarray.ConfigureCommand): + @transaction_id def do(self, argin): # checks on State, adminMode and obsState values are performed inside the # python decorators -- GitLab From d4729cd390037b2cba0ffe9b410db90a1ceed289 Mon Sep 17 00:00:00 2001 From: Gianluca Marotta Date: Mon, 21 Dec 2020 19:53:18 +0100 Subject: [PATCH 5/6] CT-206 Test that the transaction id is captured in log when applied to Configure and Assign Resources --- .../csp_lmc_common/utils/test_utils.py | 67 +++++++++++++++++ .../tests/unit/cspsubarray_unit_test.py | 73 ++++++++++++++++++- 2 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 csp-lmc-common/csp_lmc_common/utils/test_utils.py diff --git a/csp-lmc-common/csp_lmc_common/utils/test_utils.py b/csp-lmc-common/csp_lmc_common/utils/test_utils.py new file mode 100644 index 0000000..c65e226 --- /dev/null +++ b/csp-lmc-common/csp_lmc_common/utils/test_utils.py @@ -0,0 +1,67 @@ +import time +import logging +import numpy +from tango import DeviceProxy, DevState + +LOGGER = logging.getLogger(__name__) + + +class Timeout: + def __init__(self, duration): + self.endtime = time.time() + duration + + def has_expired(self): + return time.time() > self.endtime + + +class Probe: + def __init__(self, proxy, attr_name, expected_state, message): + """ + """ + self.proxy = proxy + self.attr_name = attr_name + self.expected_state = expected_state + self.message = message + self.current_state = DevState.DISABLE + + def get_attribute(self): + return self.attr_name + + def sample(self): + """ + extract the state of client and store it + """ + device_attr = self.proxy.read_attribute(self.attr_name) + self.current_state = device_attr.value + #LOGGER.info("attr_name: {} current_state:{}".format(self.attr_name, self.current_state)) + + def is_satisfied(self): + """ + Check if the state satisfies this test condition + """ + + if isinstance(self.current_state, numpy.ndarray): + return (self.expected_state == self.current_state).all() + return self.expected_state == self.current_state + + +class Poller: + def __init__(self, timeout, interval): + self.timeout = timeout + self.interval = interval + + def check(self, probe: Probe): + """ + Repeatedly check if the probe is satisfied. + Assert false when the timeout expires. + """ + timer = Timeout(self.timeout) + probe.sample() + while not probe.is_satisfied(): + if timer.has_expired(): + LOGGER.debug("Check Timeout on:{}".format(probe.get_attribute())) + assert False, probe.message + time.sleep(self.interval) + probe.sample() + LOGGER.debug("Check success on: {}".format(probe.get_attribute())) + assert True diff --git a/csp-lmc-common/tests/unit/cspsubarray_unit_test.py b/csp-lmc-common/tests/unit/cspsubarray_unit_test.py index ba6939a..e25e80e 100644 --- a/csp-lmc-common/tests/unit/cspsubarray_unit_test.py +++ b/csp-lmc-common/tests/unit/cspsubarray_unit_test.py @@ -4,13 +4,15 @@ import sys import mock import pytest import tango +import logging +import re from mock import Mock, MagicMock - -from ska.base.control_model import HealthState, ObsState +from ska.base.control_model import HealthState, ObsState, LoggingLevel from ska.base.commands import ResultCode from tango.test_context import DeviceTestContext from tango import DevState, DevFailed from csp_lmc_common.CspSubarray import CspSubarray +from csp_lmc_common.utils.test_utils import Probe, Poller def test_cspsubarray_state_and_obstate_value_after_initialization(): """ @@ -147,6 +149,57 @@ def test_cspsbarray_state_after_On_forwarded_to_subelement_subarray(): assert tango_context.device.State() == DevState.ON assert tango_context.device.obsState == ObsState.EMPTY +def test_cspsubarray_transaction_id_in_log(capsys): + """ + Test that when transaction_id decorator is applied to the Configure + and Assign Resources command, both transaction id are captured in log + """ + device_under_test = CspSubarray + cbf_subarray_fqdn = 'mid_csp_cbf/sub_elt/subarray_01' + pss_subarray_fqdn = 'mid_csp_pss/sub_elt/subarray_01' + cbf_subarray_state_attr = 'obsState' + dut_properties = { + 'CspMaster':'mid_csp/elt/master', + 'CbfSubarray': cbf_subarray_fqdn, + 'PssSubarray': 'mid_csp_pss/sub_elt/subarray_01', + } + event_subscription_map = {} + cbf_subarray_device_proxy_mock = Mock() + pss_subarray_device_proxy_mock = Mock() + proxies_to_mock = { + cbf_subarray_fqdn: cbf_subarray_device_proxy_mock, + pss_subarray_fqdn: pss_subarray_device_proxy_mock, + } + cbf_subarray_device_proxy_mock.subscribe_event.side_effect = ( + lambda attr_name, event_type, callback, *args, **kwargs: event_subscription_map.update({attr_name: callback})) + + with fake_tango_system(device_under_test, initial_dut_properties=dut_properties, proxies_to_mock=proxies_to_mock) as tango_context: + cbf_subarray_device_proxy_mock.On.side_effect = return_ok + pss_subarray_device_proxy_mock.On.side_effect = return_ok + tango_context.device.On() + assert tango_context.device.State() == DevState.ON + tango_context.device.AssignResources('{"example":"band"}') + dummy_event = create_dummy_obs_event(cbf_subarray_fqdn, ObsState.IDLE) + event_subscription_map[cbf_subarray_state_attr](dummy_event) + assert tango_context.device.obsState == ObsState.IDLE + tango_context.device.Configure('{"id":"sbi-400-scienceA"}') + # a prober is needed since the duration of the Configure command is variable. + prober_obs_state = Probe(tango_context.device, "obsState", ObsState.READY, f"Configure command out of time") + Poller(10, 0.1).check(prober_obs_state) + assert_that_log_contains('ConfigureCommand',capsys) + assert_that_log_contains('AssignResourcesCommand', capsys) + +def assert_that_log_contains(name:str,capsys): + patterns = [f'|Transaction.*(?<=Enter\[{name}\])',f'|Transaction.*(?<=Exit\[{name}\])'] + for pattern in patterns: + found = False + out, err = capsys.readouterr() + if re.match(pattern,out): + found = True + break + if not found: + raise AssertionError(f'pattern ({pattern}) not found in expected log messages') + def return_ok(): """ Return a FAILED code in the execution of a device method. @@ -187,6 +240,22 @@ def create_dummy_event(cbf_subarray_fqdn, event_value): fake_event.device.dev_name.side_effect=(lambda *args, **kwargs: mock_event_dev_name(cbf_subarray_fqdn)) return fake_event +def create_dummy_obs_event(cbf_subarray_fqdn, event_value): + """ + Create a mocked event object to test the event callback method + associate to the attribute at subscription. + param: cbf_subarray_fqdn the CBF Subarray FQDN + event_value the expected value + return: the fake event + """ + fake_event = Mock() + fake_event.err = False + fake_event.attr_name = f"{cbf_subarray_fqdn}/obsstate" + fake_event.attr_value.value = event_value + fake_event.attr_value.name = 'obsState' + fake_event.device.name = cbf_subarray_fqdn + fake_event.device.dev_name.side_effect=(lambda *args, **kwargs: mock_event_dev_name(cbf_subarray_fqdn)) + return fake_event @contextlib.contextmanager def fake_tango_system(device_under_test, initial_dut_properties={}, proxies_to_mock={}, device_proxy_import_path='tango.DeviceProxy'): -- GitLab From 0e3c46fd3de9dc3889b78782c8a77259072fc585 Mon Sep 17 00:00:00 2001 From: Gianluca Marotta Date: Tue, 22 Dec 2020 16:46:54 +0100 Subject: [PATCH 6/6] update release.py and HISTORY --- csp-lmc-common/HISTORY | 3 +++ csp-lmc-common/csp_lmc_common/release.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/csp-lmc-common/HISTORY b/csp-lmc-common/HISTORY index e2cb2ec..677ef77 100644 --- a/csp-lmc-common/HISTORY +++ b/csp-lmc-common/HISTORY @@ -1,3 +1,6 @@ +0.6.10 + - use of ska-log-transaction via transaction_id decorator. + 0.6.9: - use lmcbaseclasses 0.6.5 0.6.8: diff --git a/csp-lmc-common/csp_lmc_common/release.py b/csp-lmc-common/csp_lmc_common/release.py index dd592c5..b770d6f 100755 --- a/csp-lmc-common/csp_lmc_common/release.py +++ b/csp-lmc-common/csp_lmc_common/release.py @@ -10,7 +10,7 @@ """Release information for Python Package""" name = """csp-lmc-common""" -version = "0.6.9" +version = "0.6.10" version_info = version.split(".") description = """SKA CSP.LMC Common Software""" author = "INAF-OAA" -- GitLab