From dfbd3ca87345c9b95fe2511c872b2332cc268395 Mon Sep 17 00:00:00 2001 From: Brian Koopman Date: Thu, 26 Oct 2023 15:05:22 -0400 Subject: [PATCH 1/5] Return single active id when searching if only one --- src/sorunlib/util.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/sorunlib/util.py b/src/sorunlib/util.py index 9c03f91..84c562f 100644 --- a/src/sorunlib/util.py +++ b/src/sorunlib/util.py @@ -67,7 +67,8 @@ def _find_active_instances(agent_class): Class defined by an OCS Agent (and thus also defined in the SCF.) Returns: - list: List of instance-id's matching the given agent_class. + str or list: List of instance-id's matching the given agent_class. If + the list is of length 1, just return the only instance-id. """ cfg = load_config() @@ -84,6 +85,9 @@ def _find_active_instances(agent_class): instance_id = entry['agent_address'].split('.')[-1] instances.append(instance_id) + if len(instances) == 1: + return instances[0] + return instances @@ -116,7 +120,7 @@ def create_clients(config=None, test_mode=False): smurf_ids = _find_active_instances(smurf_agent_class) if acu_id: - acu_client = OCSClient(acu_id[0]) + acu_client = OCSClient(acu_id) clients['acu'] = acu_client # Always create smurf client list, even if empty From 3adfcdee2109a0011a150c5b2c472132fe38454b Mon Sep 17 00:00:00 2001 From: Brian Koopman Date: Thu, 26 Oct 2023 16:32:36 -0400 Subject: [PATCH 2/5] Add error handling when instantiating clients --- src/sorunlib/util.py | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/sorunlib/util.py b/src/sorunlib/util.py index 84c562f..a07b65b 100644 --- a/src/sorunlib/util.py +++ b/src/sorunlib/util.py @@ -1,9 +1,15 @@ import os +import re from sorunlib.config import load_config from ocs import site_config from ocs.ocs_client import OCSClient +from ocs.client_http import ControlClientError + + +class CrossbarConnectionError(Exception): + pass def _load_site_config(filename=None): @@ -73,7 +79,7 @@ def _find_active_instances(agent_class): """ cfg = load_config() - reg_client = OCSClient(cfg['registry']) + reg_client = _try_client(cfg['registry']) _, _, session = reg_client.main.status() instances = [] @@ -91,6 +97,28 @@ def _find_active_instances(agent_class): return instances +def _try_client(instanceid): + """User in place of OCSClient to handle common exceptions.""" + try: + client = OCSClient(instanceid) + except ControlClientError as e: + # crossbar connection error + if "Failed to connect" in str(e): + result = re.search(r"(http://[^ ]+)'", str(e)) + crossbar_url = result.group(1) + error = f"Cannot connect to crossbar server {crossbar_url}. Check your connection." + raise CrossbarConnectionError(error) + # likely an agent connection error + if "no callee registered" in str(e): + print(f"Could not instantiate OCSClient for '{instanceid}'.") + return None + # other errors, i.e. non-200 error codes + print(f"Unexpected error trying to instantiate OCSClient for '{instanceid}'.") + raise ControlClientError(e) + + return client + + def create_clients(config=None, test_mode=False): """Create all clients needed for commanding a single platform. @@ -120,11 +148,11 @@ def create_clients(config=None, test_mode=False): smurf_ids = _find_active_instances(smurf_agent_class) if acu_id: - acu_client = OCSClient(acu_id) + acu_client = _try_client(acu_id) clients['acu'] = acu_client # Always create smurf client list, even if empty - smurf_clients = [OCSClient(x) for x in smurf_ids] + smurf_clients = [_try_client(x) for x in smurf_ids] clients['smurf'] = smurf_clients return clients From 6e15c38ab72ab7e0267c861037abe56f0143f89a Mon Sep 17 00:00:00 2001 From: Brian Koopman Date: Thu, 26 Oct 2023 17:57:19 -0400 Subject: [PATCH 3/5] Reorganize construction of mock registry client --- tests/test_util.py | 151 ++++++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 72 deletions(-) diff --git a/tests/test_util.py b/tests/test_util.py index 8550b4b..e6e860c 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -7,78 +7,85 @@ os.environ["OCS_CONFIG_DIR"] = "./test_util/" -def mock_registry_client(*args, **kwargs): - """Mock out the client connection to the registry. Returning an example of - the session object that is inspected to find agent instances on the network. - - """ - client = MagicMock() - session_dict = {'session_id': 0, - 'op_name': 'main', - 'op_code': 3, - 'status': 'running', - 'success': None, - 'start_time': 1669919099.7585046, - 'end_time': None, - 'data': { - 'observatory.smurf-file-emulator-5': { - 'expired': False, - 'time_expired': None, - 'last_updated': 1669935108.8366735, - 'op_codes': { - 'uxm_setup': 1, - 'uxm_relock': 1, - 'take_iv': 1, - 'take_bias_steps': 1, - 'take_bgmap': 1, - 'bias_dets': 1, - 'take_noise': 1, - 'stream': 1}, - 'agent_class': 'SmurfFileEmulator', - 'agent_address': 'observatory.smurf-file-emulator-5'}, - 'observatory.smurf-file-emulator-7': { - 'expired': False, - 'time_expired': None, - 'last_updated': 1669935108.989246, - 'op_codes': { - 'uxm_setup': 1, - 'uxm_relock': 1, - 'take_iv': 1, - 'take_bias_steps': 1, - 'take_bgmap': 1, - 'bias_dets': 1, - 'take_noise': 1, - 'stream': 1}, - 'agent_class': 'SmurfFileEmulator', - 'agent_address': 'observatory.smurf-file-emulator-7'}, - 'observatory.fake-data-1': { - 'expired': True, - 'time_expired': None, - 'last_updated': 1669935108.989246, - 'op_codes': { - 'acq': 3, - 'count': 3, - 'set_heartbeat': 1, - 'delay_task': 1}, - 'agent_class': 'FakeDataAgent', - 'agent_address': 'observatory.fake-data-1'}, - 'observatory.acu-sat1': { - 'expired': False, - 'time_expired': None, - 'last_updated': 1669997169.469505, - 'op_codes': { - 'monitor': 3, - 'broadcast': 3, - 'generate_scan': 1, - 'go_to': 1, - 'constant_velocity_scan': 1, - 'fromfile_scan': 1, - 'set_boresight': 1, - 'stop_and_clear': 1}, - 'agent_class': 'ACUAgent', - 'agent_address': 'observatory.acu-sat1'}}} - client.main.status = MagicMock(return_value=(None, None, session_dict)) - return client +# I think this could be generalized with the Mock 'spec' argument, building an +# object that mocks a given Agent's API, but I'm not totally sure we need that +# yet, or how to make it so a different spec is loaded depending on which agent +# the client is for. +def create_mock_ocsclient(session): + def mock_client(*args, **kwargs): + client = MagicMock() + client.main.status = MagicMock(return_value=(None, None, session)) + return client + + return mock_client + + +reg_session = {'session_id': 0, + 'op_name': 'main', + 'op_code': 3, + 'status': 'running', + 'success': None, + 'start_time': 1669919099.7585046, + 'end_time': None, + 'data': { + 'observatory.smurf-file-emulator-5': { + 'expired': False, + 'time_expired': None, + 'last_updated': 1669935108.8366735, + 'op_codes': { + 'uxm_setup': 1, + 'uxm_relock': 1, + 'take_iv': 1, + 'take_bias_steps': 1, + 'take_bgmap': 1, + 'bias_dets': 1, + 'take_noise': 1, + 'stream': 1}, + 'agent_class': 'SmurfFileEmulator', + 'agent_address': 'observatory.smurf-file-emulator-5'}, + 'observatory.smurf-file-emulator-7': { + 'expired': False, + 'time_expired': None, + 'last_updated': 1669935108.989246, + 'op_codes': { + 'uxm_setup': 1, + 'uxm_relock': 1, + 'take_iv': 1, + 'take_bias_steps': 1, + 'take_bgmap': 1, + 'bias_dets': 1, + 'take_noise': 1, + 'stream': 1}, + 'agent_class': 'SmurfFileEmulator', + 'agent_address': 'observatory.smurf-file-emulator-7'}, + 'observatory.fake-data-1': { + 'expired': True, + 'time_expired': None, + 'last_updated': 1669935108.989246, + 'op_codes': { + 'acq': 3, + 'count': 3, + 'set_heartbeat': 1, + 'delay_task': 1}, + 'agent_class': 'FakeDataAgent', + 'agent_address': 'observatory.fake-data-1'}, + 'observatory.acu-sat1': { + 'expired': False, + 'time_expired': None, + 'last_updated': 1669997169.469505, + 'op_codes': { + 'monitor': 3, + 'broadcast': 3, + 'generate_scan': 1, + 'go_to': 1, + 'constant_velocity_scan': 1, + 'fromfile_scan': 1, + 'set_boresight': 1, + 'stop_and_clear': 1}, + 'agent_class': 'ACUAgent', + 'agent_address': 'observatory.acu-sat1'}}} + +mock_registry_client = create_mock_ocsclient(reg_session) def test_load_site_config(): From 7dacbccceaf0fd2999934ada0bd12c90b3d5425b Mon Sep 17 00:00:00 2001 From: Brian Koopman Date: Thu, 26 Oct 2023 17:57:31 -0400 Subject: [PATCH 4/5] Update registry_address in test config --- tests/test_util/default.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_util/default.yaml b/tests/test_util/default.yaml index 3e7c082..2fb4e42 100644 --- a/tests/test_util/default.yaml +++ b/tests/test_util/default.yaml @@ -3,7 +3,7 @@ hub: wamp_http: http://localhost:8001/call wamp_realm: test_realm address_root: observatory - registry_address: observatory.registry + registry_address: registry hosts: localhost: { From 71edd71a5f3c888e5fb9d430bbcdbaff5ad1c920 Mon Sep 17 00:00:00 2001 From: Brian Koopman Date: Fri, 27 Oct 2023 11:40:18 -0400 Subject: [PATCH 5/5] Test error states of _try_client() --- tests/test_util.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_util.py b/tests/test_util.py index e6e860c..1972b11 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -1,8 +1,11 @@ import os +import pytest +from ocs.client_http import ControlClientError from unittest.mock import MagicMock, patch from sorunlib import util +from sorunlib.util import CrossbarConnectionError os.environ["OCS_CONFIG_DIR"] = "./test_util/" @@ -88,6 +91,16 @@ def mock_client(*args, **kwargs): mock_registry_client = create_mock_ocsclient(reg_session) +class NoAgentClient: + def __init__(self, *args, **kwargs): + raise ControlClientError("no callee registered for procedure fake.agent.op") + + +class UnexpectedErrorClient: + def __init__(self, *args, **kwargs): + raise ControlClientError("Server replied with code 500") + + def test_load_site_config(): cfg = util._load_site_config() assert 'localhost' in cfg.hosts @@ -118,6 +131,27 @@ def test_find_active_instances_expired(): assert 'fake-data-1' not in instances +def test__try_client_no_crossbar_connection(): + """This test assumes a crossbar server isn't running at + http://localhost:8001. It tests that we raise an error when trying to + connect to the registry to scan for agents if crossbar is unavailable.""" + with pytest.raises(CrossbarConnectionError): + util._try_client('test-agent') + + +@patch('sorunlib.util.OCSClient', NoAgentClient) +def test__try_client_no_agent_connection(): + """This tests that we get None back when the agent is offline.""" + client = util._try_client('test-agent') + assert client is None + + +@patch('sorunlib.util.OCSClient', UnexpectedErrorClient) +def test__try_client_other_error(): + with pytest.raises(ControlClientError): + util._try_client('test-agent') + + @patch('sorunlib.util.OCSClient', mock_registry_client) def test_create_clients(): clients = util.create_clients()