Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add error handling to client instantiation #104

Merged
merged 5 commits into from
Nov 27, 2023
Merged

Add error handling to client instantiation #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dfbd3ca
Return single active id when searching if only one
BrianJKoopman Oct 26, 2023
3adfcde
Add error handling when instantiating clients
BrianJKoopman Oct 26, 2023
6e15c38
Reorganize construction of mock registry client
BrianJKoopman Oct 26, 2023
7dacbcc
Update registry_address in test config
BrianJKoopman Oct 26, 2023
71edd71
Test error states of _try_client()
BrianJKoopman Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions src/sorunlib/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -67,12 +73,13 @@ 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()

reg_client = OCSClient(cfg['registry'])
reg_client = _try_client(cfg['registry'])
_, _, session = reg_client.main.status()

instances = []
Expand All @@ -84,9 +91,34 @@ 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


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.

Expand Down Expand Up @@ -116,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[0])
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
185 changes: 113 additions & 72 deletions tests/test_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,84 +1,104 @@
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/"


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)


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():
Expand Down Expand Up @@ -111,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()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_util/default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down