From f27587ef55cd8da23e8c1d7f935a1f7a1dc4932a Mon Sep 17 00:00:00 2001 From: Patrik Marschalik Date: Thu, 20 Sep 2018 16:18:19 +0100 Subject: [PATCH 1/7] Check for time shift between instrument and webserver --- .../instrument_information_collator.py | 2 +- external_webpage/request_handler_utils.py | 39 ++++++++++++++++++- front_end/default.html | 9 ++++- front_end/display_blocks.js | 17 +++++++- webserver.py | 6 ++- 5 files changed, 67 insertions(+), 6 deletions(-) diff --git a/external_webpage/instrument_information_collator.py b/external_webpage/instrument_information_collator.py index 386b092..58fc621 100644 --- a/external_webpage/instrument_information_collator.py +++ b/external_webpage/instrument_information_collator.py @@ -118,7 +118,7 @@ def _get_inst_pvs(self, ans, blocks_all): "RAWFRAMES", "RAWFRAMES_PD", "PERIOD", "NUMPERIODS", "PERIODSEQ", "BEAMCURRENT", "TOTALUAMPS", "COUNTRATE", "DAEMEMORYUSED", "TOTALCOUNTS", "DAETIMINGSOURCE", "MONITORCOUNTS", "MONITORSPECTRUM", "MONITORFROM", "MONITORTO", "NUMTIMECHANNELS", "NUMSPECTRA", "SHUTTER", - "SIM_MODE"] + "SIM_MODE", "TIME_OF_DAY"] try: set_rc_values_for_blocks(blocks_all.values(), ans) diff --git a/external_webpage/request_handler_utils.py b/external_webpage/request_handler_utils.py index 1a22058..bda165c 100644 --- a/external_webpage/request_handler_utils.py +++ b/external_webpage/request_handler_utils.py @@ -1,5 +1,6 @@ import re from collections import OrderedDict +import time def get_instrument_and_callback(path): @@ -51,15 +52,51 @@ def get_summary_details_of_all_instruments(data): return inst_data -def get_detailed_state_of_specific_instrument(instrument, data): +def get_instrument_time_since_epoch(instrument_data): + """ + Return the instrument time as seconds sind epoch. + """ + + inst_time_str = instrument_data['inst_pvs']['TIME_OF_DAY']['value'] + inst_time_struct = time.strptime(inst_time_str, '%m/%d/%Y %H:%M:%S') + inst_time = time.mktime(inst_time_struct) + + return inst_time + + +def check_outdated(instrument_data, time_shift_threshold): + """ + Update the instrument data with the time shift to the webserver. + :param instrument_data: The data dictionary of an instrument + :param time_shift_threshold: If the time shift is greater than this value the data is considered outdated + """ + try: + inst_time = get_instrument_time_since_epoch(instrument_data) + current_time = time.time() + time_diff = abs(current_time - inst_time) + except: + time_diff = None + + instrument_data['time_diff'] = time_diff + + if time_diff is not None and time_diff > time_shift_threshold: + instrument_data['outdated'] = True + else: + instrument_data['outdated'] = False + + +def get_detailed_state_of_specific_instrument(instrument, data, time_shift_threshold): """ Gets the detailed state of a specific instrument, used to display the instrument's dataweb screen :param instrument: The instrument to get data for :param data: The data scraped from the archiver webpage + :param time_shift_threshold: The allowed time difference in seconds between the instrument and the webserver time :return: The data from the archiver webpage filtered to only contain data about the requested instrument """ if instrument not in data.keys(): raise ValueError(str(instrument) + " not known") if data[instrument] == "": raise ValueError("Instrument has become unavailable") + check_outdated(data[instrument], time_shift_threshold) + return data[instrument] diff --git a/front_end/default.html b/front_end/default.html index 2e9f2e4..a71a268 100644 --- a/front_end/default.html +++ b/front_end/default.html @@ -16,8 +16,15 @@
-
+ + + +
+

+ 		+

+

Run Information

diff --git a/front_end/display_blocks.js b/front_end/display_blocks.js index cacbe0a..b9d2bdc 100644 --- a/front_end/display_blocks.js +++ b/front_end/display_blocks.js @@ -4,6 +4,7 @@ var HOST = "http://dataweb.isis.rl.ac.uk" var instrument = getURLParameter("Instrument"); var nodeInstTitle = document.createElement("H2"); var nodeConfigTitle = document.createElement("H2"); +var nodeTimeDiffTitle = document.createElement("H2"); var instrumentState; var showHidden; var timeout = 4000; @@ -37,8 +38,9 @@ dictInstPV = { MONITORTO: 'Monitor To', NUMTIMECHANNELS: 'Number of Time Channels', NUMSPECTRA: 'Number of Spectra', - SHUTTER: 'Shutter Status', - SIM_MODE: 'DAE Simulation mode' + SHUTTER: 'Shutter Status', + SIM_MODE: 'DAE Simulation mode', + TIME_OF_DAY: 'Instrument time', }; /** @@ -141,6 +143,7 @@ function parseObject(obj) { showHidden = document.getElementById("showHidden").checked; clear(nodeInstTitle); clear(nodeConfigTitle); + clear(nodeTimeDiffTitle); // populate blocks var nodeGroups = document.getElementById("groups"); @@ -152,6 +155,8 @@ function parseObject(obj) { getDisplayRunInfo(nodeInstPVs, instrumentState.inst_pvs); + getDisplayTimeDiffInfo(instrumentState); + nodeInstTitle.appendChild(document.createTextNode(instrument)); nodeConfigTitle.appendChild(document.createTextNode("Configuration: " + instrumentState.config_name)); @@ -165,6 +170,14 @@ function clearBox(elementID){ document.getElementById(elementID).innerHTML = ""; } +function getDisplayTimeDiffInfo(instrumentState){ + if (instrumentState.outdated) { + nodeTimeDiffTitle.appendChild(document.createTextNode("There is a time shift of " + instrumentState.time_diff + " seconds between the instrument and the web server. Dataweb may not be updating correctly.")); + document.getElementById("time_diff").appendChild(nodeTimeDiffTitle); + document.getElementById("time_diff").style.color = "RED"; + } +} + /** * creates a Title at the top looking similar to the IBEX GUI */ diff --git a/webserver.py b/webserver.py index c714c65..758c3c0 100644 --- a/webserver.py +++ b/webserver.py @@ -19,6 +19,10 @@ HOST, PORT = '', 60000 +# If the instrument time differs by the webserver time by more than +# TIME_SHIFT_THRESHOLD seconds, then this should be reported web dashboard. +TIME_SHIFT_THRESHOLD = 5 * 3600 + class MyHandler(BaseHTTPRequestHandler): """ @@ -43,7 +47,7 @@ def do_GET(self): "instruments": get_summary_details_of_all_instruments(scraped_data)} else: - ans = get_detailed_state_of_specific_instrument(instrument, scraped_data) + ans = get_detailed_state_of_specific_instrument(instrument, scraped_data, TIME_SHIFT_THRESHOLD) try: ans_as_json = str(json.dumps(ans)) From 00605f59cde01277cbee5311fd039f6817b68397 Mon Sep 17 00:00:00 2001 From: Patrik Marschalik Date: Fri, 21 Sep 2018 10:59:28 +0100 Subject: [PATCH 2/7] Fix threshold value --- webserver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webserver.py b/webserver.py index 758c3c0..1cda9c3 100644 --- a/webserver.py +++ b/webserver.py @@ -21,7 +21,7 @@ # If the instrument time differs by the webserver time by more than # TIME_SHIFT_THRESHOLD seconds, then this should be reported web dashboard. -TIME_SHIFT_THRESHOLD = 5 * 3600 +TIME_SHIFT_THRESHOLD = 5 * 60 class MyHandler(BaseHTTPRequestHandler): From bca9e892b0c4ae039a0483fb4ee502d08b42b0af Mon Sep 17 00:00:00 2001 From: Patrik Marschalik Date: Fri, 21 Sep 2018 15:55:16 +0100 Subject: [PATCH 3/7] Refactoring --- external_webpage/request_handler_utils.py | 54 +++++++++++++++++------ front_end/display_blocks.js | 2 +- webserver.py | 6 +-- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/external_webpage/request_handler_utils.py b/external_webpage/request_handler_utils.py index bda165c..5ad58ef 100644 --- a/external_webpage/request_handler_utils.py +++ b/external_webpage/request_handler_utils.py @@ -1,6 +1,10 @@ import re from collections import OrderedDict import time +import logging + + +logger = logging.getLogger('JSON_bourne') def get_instrument_and_callback(path): @@ -54,38 +58,60 @@ def get_summary_details_of_all_instruments(data): def get_instrument_time_since_epoch(instrument_data): """ - Return the instrument time as seconds sind epoch. + Return the instrument time as seconds since epoch. + + Raises: KeyError: instrument time cannot be parsed from instrument_data + Raises: ValueError: if instrument time has wrong time format """ - inst_time_str = instrument_data['inst_pvs']['TIME_OF_DAY']['value'] - inst_time_struct = time.strptime(inst_time_str, '%m/%d/%Y %H:%M:%S') + time_format = '%m/%d/%Y %H:%M:%S' + try: + inst_time_str = instrument_data['inst_pvs']['TIME_OF_DAY']['value'] + except KeyError: + logger.error("Cannot parse TIME_OF_DAY from instrument data.") + raise + + try: + inst_time_struct = time.strptime(inst_time_str, time_format) + except ValueError: + logger.error("Instrument time does not match format {}.".format(time_format)) + raise + inst_time = time.mktime(inst_time_struct) return inst_time -def check_outdated(instrument_data, time_shift_threshold): +def check_out_of_sync(instrument_data, time_shift_threshold, extract_time_from_instrument_func=get_instrument_time_since_epoch, current_time_func=time.time): """ Update the instrument data with the time shift to the webserver. + :param instrument_data: The data dictionary of an instrument :param time_shift_threshold: If the time shift is greater than this value the data is considered outdated + + Raises: ValueError: if time_diff cannot calculated + Raises: TypeError: if time_diff cannot calculated """ try: - inst_time = get_instrument_time_since_epoch(instrument_data) - current_time = time.time() + inst_time = extract_time_from_instrument_func(instrument_data) + current_time = current_time_func() time_diff = abs(current_time - inst_time) - except: + except (ValueError, TypeError, KeyError): time_diff = None - instrument_data['time_diff'] = time_diff + try: + instrument_data['time_diff'] = time_diff - if time_diff is not None and time_diff > time_shift_threshold: - instrument_data['outdated'] = True - else: - instrument_data['outdated'] = False + if time_diff is not None and time_diff > time_shift_threshold: + instrument_data['out_of_sync'] = True + logger.warning("There is a time shift of {} seconds between insrument and web server".format(time_diff)) + else: + instrument_data['out_of_sync'] = False + except TypeError: + logger.error("Cannot set time shift information.") -def get_detailed_state_of_specific_instrument(instrument, data, time_shift_threshold): +def get_detailed_state_of_specific_instrument(instrument, data, time_shift_threshold=5*60): """ Gets the detailed state of a specific instrument, used to display the instrument's dataweb screen :param instrument: The instrument to get data for @@ -97,6 +123,6 @@ def get_detailed_state_of_specific_instrument(instrument, data, time_shift_thres raise ValueError(str(instrument) + " not known") if data[instrument] == "": raise ValueError("Instrument has become unavailable") - check_outdated(data[instrument], time_shift_threshold) + check_out_of_sync(data[instrument], time_shift_threshold) return data[instrument] diff --git a/front_end/display_blocks.js b/front_end/display_blocks.js index b9d2bdc..00b35d7 100644 --- a/front_end/display_blocks.js +++ b/front_end/display_blocks.js @@ -171,7 +171,7 @@ function clearBox(elementID){ } function getDisplayTimeDiffInfo(instrumentState){ - if (instrumentState.outdated) { + if (instrumentState.out_of_sync) { nodeTimeDiffTitle.appendChild(document.createTextNode("There is a time shift of " + instrumentState.time_diff + " seconds between the instrument and the web server. Dataweb may not be updating correctly.")); document.getElementById("time_diff").appendChild(nodeTimeDiffTitle); document.getElementById("time_diff").style.color = "RED"; diff --git a/webserver.py b/webserver.py index 1cda9c3..c714c65 100644 --- a/webserver.py +++ b/webserver.py @@ -19,10 +19,6 @@ HOST, PORT = '', 60000 -# If the instrument time differs by the webserver time by more than -# TIME_SHIFT_THRESHOLD seconds, then this should be reported web dashboard. -TIME_SHIFT_THRESHOLD = 5 * 60 - class MyHandler(BaseHTTPRequestHandler): """ @@ -47,7 +43,7 @@ def do_GET(self): "instruments": get_summary_details_of_all_instruments(scraped_data)} else: - ans = get_detailed_state_of_specific_instrument(instrument, scraped_data, TIME_SHIFT_THRESHOLD) + ans = get_detailed_state_of_specific_instrument(instrument, scraped_data) try: ans_as_json = str(json.dumps(ans)) From 25abb6afdc4ceeb860ded09d33c28434b8a15eb3 Mon Sep 17 00:00:00 2001 From: Patrik Marschalik Date: Fri, 21 Sep 2018 16:03:34 +0100 Subject: [PATCH 4/7] Add unit tests --- tests/test_handler_utils.py | 71 ++++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/tests/test_handler_utils.py b/tests/test_handler_utils.py index fe2e434..b6a2e62 100644 --- a/tests/test_handler_utils.py +++ b/tests/test_handler_utils.py @@ -1,9 +1,11 @@ from hamcrest import * from external_webpage.request_handler_utils import get_instrument_and_callback, \ - get_summary_details_of_all_instruments, get_detailed_state_of_specific_instrument + get_summary_details_of_all_instruments, get_detailed_state_of_specific_instrument, \ + get_instrument_time_since_epoch, check_out_of_sync import json import unittest +import time CALLBACK_STR = "?callback={}&" @@ -161,3 +163,70 @@ def test_GIVEN_instrument_with_data_WHEN_get_details_called_THEN_data_returned_f out = get_detailed_state_of_specific_instrument(inst, data_dict) self.assertEqual(out, inst_data) + + +class TestHandlerUtils_InstrumentTime(unittest.TestCase): + + def test_that_GIVEN_good_instrument_THEN_retrun_inst_time(self): + instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': '01/01/1970 01:00:00'}}} + inst_time = get_instrument_time_since_epoch(instrument_data) + + assert_that(inst_time, equal_to(3600.0)) + + def test_that_GIVEN_wrong_formated_instrument_THEN_raise_value_error(self): + instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'this is no propper time format'}}} + + assert_that(calling(get_instrument_time_since_epoch).with_args(instrument_data), raises(ValueError)) + + def test_that_GIVEN_instrument_data_without_time_of_day_THEN_raise_value_error(self): + instrument_data = {'inst_pvs': {'this_is_not_TIME_OF_DAY': {'value': '01/01/1970 01:00:00'}}} + + assert_that(calling(get_instrument_time_since_epoch).with_args(instrument_data), raises(KeyError)) + + +class TestHandlerUtils_CheckOutOfSync(unittest.TestCase): + + def test_that_GIVEN_time_difference_is_greater_than_threshold_THEN_set_out_of_sync_to_true(self): + instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} + + check_out_of_sync(instrument_data, time_shift_threshold=2, + extract_time_from_instrument_func=lambda x : 5, + current_time_func=lambda : 10) + + assert_that(instrument_data['out_of_sync'], equal_to(True)) + + def test_that_GIVEN_time_difference_is_less_than_threshold_THEN_set_out_of_sync_to_false(self): + instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} + + check_out_of_sync(instrument_data, time_shift_threshold=17, + extract_time_from_instrument_func=lambda x : 5, + current_time_func=lambda : 10) + + assert_that(instrument_data['out_of_sync'], equal_to(False)) + + def test_that_GIVEN_time_difference_of_five_THEN_set_time_diff_to_five(self): + instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} + + check_out_of_sync(instrument_data, time_shift_threshold=17, + extract_time_from_instrument_func=lambda x : 5, + current_time_func=lambda : 10) + + assert_that(instrument_data['time_diff'], equal_to(5)) + + def test_that_GIVEN_invalid_time_THEN_set_time_diff_to_None(self): + instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} + + check_out_of_sync(instrument_data, time_shift_threshold=17, + extract_time_from_instrument_func=lambda x : 'foo', + current_time_func=lambda : 10) + + assert_that(instrument_data['time_diff'], equal_to(None)) + + def test_that_GIVEN_invalid_time_THEN_set_out_of_sync_to_false(self): + instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} + + check_out_of_sync(instrument_data, time_shift_threshold=17, + extract_time_from_instrument_func=lambda x : 'foo', + current_time_func=lambda : 10) + + assert_that(instrument_data['out_of_sync'], equal_to(False)) From 75e2f1de3676c3f23ee95212d92243457f349e55 Mon Sep 17 00:00:00 2001 From: Patrik Marschalik Date: Wed, 26 Sep 2018 11:47:30 +0200 Subject: [PATCH 5/7] Fix some small bits --- external_webpage/request_handler_utils.py | 29 +++++++++++++++-------- tests/test_handler_utils.py | 24 +++++++++---------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/external_webpage/request_handler_utils.py b/external_webpage/request_handler_utils.py index 5ad58ef..2efa622 100644 --- a/external_webpage/request_handler_utils.py +++ b/external_webpage/request_handler_utils.py @@ -64,38 +64,47 @@ def get_instrument_time_since_epoch(instrument_data): Raises: ValueError: if instrument time has wrong time format """ + try: + channel = instrument_data['Channel'] + except KeyError: + channel = "UNKNOWN" + time_format = '%m/%d/%Y %H:%M:%S' try: - inst_time_str = instrument_data['inst_pvs']['TIME_OF_DAY']['value'] + tod = 'TIME_OF_DAY' + inst_time_str = instrument_data['inst_pvs'][tod]['value'] except KeyError: - logger.error("Cannot parse TIME_OF_DAY from instrument data.") + logger.exception("Cannot find {} in PV {}.".format(tod, channel)) raise try: inst_time_struct = time.strptime(inst_time_str, time_format) except ValueError: - logger.error("Instrument time does not match format {}.".format(time_format)) + logger.exception("Value {} from PV {} does not match time format {}.".format(inst_time_str, channel, time_format)) raise - inst_time = time.mktime(inst_time_struct) + try: + inst_time = time.mktime(inst_time_struct) + except (ValueError, OverflowError): + inst_time = None + logger.error("Cannot parse value {} from PV {} as time".format(inst_time, channel)) return inst_time -def check_out_of_sync(instrument_data, time_shift_threshold, extract_time_from_instrument_func=get_instrument_time_since_epoch, current_time_func=time.time): +def set_time_shift(instrument_data, time_shift_threshold, + extract_time_from_instrument_func=get_instrument_time_since_epoch, + current_time_func=time.time): """ Update the instrument data with the time shift to the webserver. :param instrument_data: The data dictionary of an instrument :param time_shift_threshold: If the time shift is greater than this value the data is considered outdated - - Raises: ValueError: if time_diff cannot calculated - Raises: TypeError: if time_diff cannot calculated """ try: inst_time = extract_time_from_instrument_func(instrument_data) current_time = current_time_func() - time_diff = abs(current_time - inst_time) + time_diff = int(round(abs(current_time - inst_time))) except (ValueError, TypeError, KeyError): time_diff = None @@ -123,6 +132,6 @@ def get_detailed_state_of_specific_instrument(instrument, data, time_shift_thres raise ValueError(str(instrument) + " not known") if data[instrument] == "": raise ValueError("Instrument has become unavailable") - check_out_of_sync(data[instrument], time_shift_threshold) + set_time_shift(data[instrument], time_shift_threshold) return data[instrument] diff --git a/tests/test_handler_utils.py b/tests/test_handler_utils.py index b6a2e62..bf7b72d 100644 --- a/tests/test_handler_utils.py +++ b/tests/test_handler_utils.py @@ -2,7 +2,7 @@ from external_webpage.request_handler_utils import get_instrument_and_callback, \ get_summary_details_of_all_instruments, get_detailed_state_of_specific_instrument, \ - get_instrument_time_since_epoch, check_out_of_sync + get_instrument_time_since_epoch, set_time_shift import json import unittest import time @@ -171,7 +171,7 @@ def test_that_GIVEN_good_instrument_THEN_retrun_inst_time(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': '01/01/1970 01:00:00'}}} inst_time = get_instrument_time_since_epoch(instrument_data) - assert_that(inst_time, equal_to(3600.0)) + assert_that(inst_time, equal_to(3600.0 + time.timezone)) def test_that_GIVEN_wrong_formated_instrument_THEN_raise_value_error(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'this is no propper time format'}}} @@ -189,44 +189,44 @@ class TestHandlerUtils_CheckOutOfSync(unittest.TestCase): def test_that_GIVEN_time_difference_is_greater_than_threshold_THEN_set_out_of_sync_to_true(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - check_out_of_sync(instrument_data, time_shift_threshold=2, + set_time_shift(instrument_data, time_shift_threshold=2, extract_time_from_instrument_func=lambda x : 5, current_time_func=lambda : 10) - assert_that(instrument_data['out_of_sync'], equal_to(True)) + assert_that(instrument_data['out_of_sync'], equal_to(True)) def test_that_GIVEN_time_difference_is_less_than_threshold_THEN_set_out_of_sync_to_false(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - check_out_of_sync(instrument_data, time_shift_threshold=17, + set_time_shift(instrument_data, time_shift_threshold=17, extract_time_from_instrument_func=lambda x : 5, current_time_func=lambda : 10) - assert_that(instrument_data['out_of_sync'], equal_to(False)) + assert_that(instrument_data['out_of_sync'], equal_to(False)) def test_that_GIVEN_time_difference_of_five_THEN_set_time_diff_to_five(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - check_out_of_sync(instrument_data, time_shift_threshold=17, + set_time_shift(instrument_data, time_shift_threshold=17, extract_time_from_instrument_func=lambda x : 5, current_time_func=lambda : 10) - assert_that(instrument_data['time_diff'], equal_to(5)) + assert_that(instrument_data['time_diff'], equal_to(5)) def test_that_GIVEN_invalid_time_THEN_set_time_diff_to_None(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - check_out_of_sync(instrument_data, time_shift_threshold=17, + set_time_shift(instrument_data, time_shift_threshold=17, extract_time_from_instrument_func=lambda x : 'foo', current_time_func=lambda : 10) - assert_that(instrument_data['time_diff'], equal_to(None)) + assert_that(instrument_data['time_diff'], equal_to(None)) def test_that_GIVEN_invalid_time_THEN_set_out_of_sync_to_false(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - check_out_of_sync(instrument_data, time_shift_threshold=17, + set_time_shift(instrument_data, time_shift_threshold=17, extract_time_from_instrument_func=lambda x : 'foo', current_time_func=lambda : 10) - assert_that(instrument_data['out_of_sync'], equal_to(False)) + assert_that(instrument_data['out_of_sync'], equal_to(False)) From 394cd0288c94b244d79cc6ada2c1014e51016500 Mon Sep 17 00:00:00 2001 From: Dominic Oram Date: Tue, 19 Oct 2021 16:15:31 +0100 Subject: [PATCH 6/7] Added instrument name to logging --- external_webpage/request_handler_utils.py | 31 +++++++++++++---------- tests/test_handler_utils.py | 24 +++++++++--------- webserver.py | 2 +- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/external_webpage/request_handler_utils.py b/external_webpage/request_handler_utils.py index 313c23e..002713f 100644 --- a/external_webpage/request_handler_utils.py +++ b/external_webpage/request_handler_utils.py @@ -4,7 +4,6 @@ import time import logging - logger = logging.getLogger('JSON_bourne') @@ -57,12 +56,16 @@ def get_summary_details_of_all_instruments(data): return inst_data -def get_instrument_time_since_epoch(instrument_data): +def get_instrument_time_since_epoch(instrument_name, instrument_data): """ Return the instrument time as seconds since epoch. - Raises: KeyError: instrument time cannot be parsed from instrument_data - Raises: ValueError: if instrument time has wrong time format + :param instrument_name: The name of the instrument + :param instrument_data: The data associated with the instrument + :return: the instrument time as seconds since epoch. + + :raises: KeyError: instrument time cannot be parsed from instrument_data + :raises: ValueError: if instrument time has wrong time format """ try: @@ -75,35 +78,37 @@ def get_instrument_time_since_epoch(instrument_data): tod = 'TIME_OF_DAY' inst_time_str = instrument_data['inst_pvs'][tod]['value'] except KeyError: - logger.exception("Cannot find {} in PV {}.".format(tod, channel)) + logger.exception(f"{instrument_name}: Cannot find {tod} in PV {channel}.") raise try: inst_time_struct = time.strptime(inst_time_str, time_format) except ValueError: - logger.exception("Value {} from PV {} does not match time format {}.".format(inst_time_str, channel, time_format)) + logger.exception(f"{instrument_name}: Value {inst_time_str} from PV {channel} does not match time format " + f"{time_format}.") raise try: inst_time = time.mktime(inst_time_struct) except (ValueError, OverflowError): inst_time = None - logger.error("Cannot parse value {} from PV {} as time".format(inst_time, channel)) + logger.error(f"{instrument_name}: Cannot parse value {inst_time} from PV {channel} as time") return inst_time -def set_time_shift(instrument_data, time_shift_threshold, +def set_time_shift(instrument_name, instrument_data, time_shift_threshold, extract_time_from_instrument_func=get_instrument_time_since_epoch, current_time_func=time.time): """ Update the instrument data with the time shift to the webserver. - :param instrument_data: The data dictionary of an instrument + :param instrument_name: The name of the instrument + :param instrument_data: The data dictionary of the instrument :param time_shift_threshold: If the time shift is greater than this value the data is considered outdated """ try: - inst_time = extract_time_from_instrument_func(instrument_data) + inst_time = extract_time_from_instrument_func(instrument_name, instrument_data) current_time = current_time_func() time_diff = int(round(abs(current_time - inst_time))) except (ValueError, TypeError, KeyError): @@ -114,11 +119,11 @@ def set_time_shift(instrument_data, time_shift_threshold, if time_diff is not None and time_diff > time_shift_threshold: instrument_data['out_of_sync'] = True - logger.warning("There is a time shift of {} seconds between insrument and web server".format(time_diff)) + logger.warning(f"There is a time shift of {time_diff} seconds between {instrument_name} and web server") else: instrument_data['out_of_sync'] = False except TypeError: - logger.error("Cannot set time shift information.") + logger.error(f"Cannot set time shift information for {instrument_name}.") def get_detailed_state_of_specific_instrument(instrument, data, time_shift_threshold=5*60): @@ -133,6 +138,6 @@ def get_detailed_state_of_specific_instrument(instrument, data, time_shift_thres raise ValueError(str(instrument) + " not known") if data[instrument] == "": raise ValueError("Instrument has become unavailable") - set_time_shift(data[instrument], time_shift_threshold) + set_time_shift(instrument, data[instrument], time_shift_threshold) return data[instrument] diff --git a/tests/test_handler_utils.py b/tests/test_handler_utils.py index 34563cb..f017353 100644 --- a/tests/test_handler_utils.py +++ b/tests/test_handler_utils.py @@ -169,19 +169,19 @@ class TestHandlerUtils_InstrumentTime(unittest.TestCase): def test_that_GIVEN_good_instrument_THEN_retrun_inst_time(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': '01/01/1970 01:00:00'}}} - inst_time = get_instrument_time_since_epoch(instrument_data) + inst_time = get_instrument_time_since_epoch("", instrument_data) assert_that(inst_time, equal_to(3600.0 + time.timezone)) def test_that_GIVEN_wrong_formated_instrument_THEN_raise_value_error(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'this is no propper time format'}}} - assert_that(calling(get_instrument_time_since_epoch).with_args(instrument_data), raises(ValueError)) + assert_that(calling(get_instrument_time_since_epoch).with_args("", instrument_data), raises(ValueError)) def test_that_GIVEN_instrument_data_without_time_of_day_THEN_raise_value_error(self): instrument_data = {'inst_pvs': {'this_is_not_TIME_OF_DAY': {'value': '01/01/1970 01:00:00'}}} - assert_that(calling(get_instrument_time_since_epoch).with_args(instrument_data), raises(KeyError)) + assert_that(calling(get_instrument_time_since_epoch).with_args("", instrument_data), raises(KeyError)) class TestHandlerUtils_CheckOutOfSync(unittest.TestCase): @@ -189,7 +189,7 @@ class TestHandlerUtils_CheckOutOfSync(unittest.TestCase): def test_that_GIVEN_time_difference_is_greater_than_threshold_THEN_set_out_of_sync_to_true(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - set_time_shift(instrument_data, time_shift_threshold=2, + set_time_shift("", instrument_data, time_shift_threshold=2, extract_time_from_instrument_func=lambda x : 5, current_time_func=lambda : 10) @@ -198,7 +198,7 @@ def test_that_GIVEN_time_difference_is_greater_than_threshold_THEN_set_out_of_sy def test_that_GIVEN_time_difference_is_less_than_threshold_THEN_set_out_of_sync_to_false(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - set_time_shift(instrument_data, time_shift_threshold=17, + set_time_shift("", instrument_data, time_shift_threshold=17, extract_time_from_instrument_func=lambda x : 5, current_time_func=lambda : 10) @@ -207,7 +207,7 @@ def test_that_GIVEN_time_difference_is_less_than_threshold_THEN_set_out_of_sync_ def test_that_GIVEN_time_difference_of_five_THEN_set_time_diff_to_five(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - set_time_shift(instrument_data, time_shift_threshold=17, + set_time_shift("", instrument_data, time_shift_threshold=17, extract_time_from_instrument_func=lambda x : 5, current_time_func=lambda : 10) @@ -216,17 +216,17 @@ def test_that_GIVEN_time_difference_of_five_THEN_set_time_diff_to_five(self): def test_that_GIVEN_invalid_time_THEN_set_time_diff_to_None(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - set_time_shift(instrument_data, time_shift_threshold=17, - extract_time_from_instrument_func=lambda x : 'foo', - current_time_func=lambda : 10) + set_time_shift("", instrument_data, time_shift_threshold=17, + extract_time_from_instrument_func=lambda x: 'foo', + current_time_func=lambda: 10) assert_that(instrument_data['time_diff'], equal_to(None)) def test_that_GIVEN_invalid_time_THEN_set_out_of_sync_to_false(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} - set_time_shift(instrument_data, time_shift_threshold=17, - extract_time_from_instrument_func=lambda x : 'foo', - current_time_func=lambda : 10) + set_time_shift("", instrument_data, time_shift_threshold=17, + extract_time_from_instrument_func=lambda x: 'foo', + current_time_func=lambda: 10) assert_that(instrument_data['out_of_sync'], equal_to(False)) diff --git a/webserver.py b/webserver.py index af6278e..b380053 100644 --- a/webserver.py +++ b/webserver.py @@ -80,7 +80,7 @@ def log_message(self, format, *args): if __name__ == '__main__': # It can sometime be useful to define a local instrument list to add/override the instrument list do this here - # E.g. to add local instrument local_inst_list = {"localhost": "localhost"} + # E.g. to add local instrument local_inst_list = {"LOCALHOST": ("localhost", "MYPVPREFIX")} local_inst_list = {} web_manager = WebScrapperManager(local_inst_list=local_inst_list) web_manager.start() From 9327d1804ac1c908486d13e6eea6995d430130e5 Mon Sep 17 00:00:00 2001 From: JamesKingWork Date: Tue, 26 Oct 2021 13:18:06 +0000 Subject: [PATCH 7/7] Use lambda with correct amount of parameters to fix tests, use underscore convention for unused parameter --- tests/test_handler_utils.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_handler_utils.py b/tests/test_handler_utils.py index f017353..f2c5778 100644 --- a/tests/test_handler_utils.py +++ b/tests/test_handler_utils.py @@ -190,7 +190,7 @@ def test_that_GIVEN_time_difference_is_greater_than_threshold_THEN_set_out_of_sy instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} set_time_shift("", instrument_data, time_shift_threshold=2, - extract_time_from_instrument_func=lambda x : 5, + extract_time_from_instrument_func=lambda _, __ : 5, current_time_func=lambda : 10) assert_that(instrument_data['out_of_sync'], equal_to(True)) @@ -199,7 +199,7 @@ def test_that_GIVEN_time_difference_is_less_than_threshold_THEN_set_out_of_sync_ instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} set_time_shift("", instrument_data, time_shift_threshold=17, - extract_time_from_instrument_func=lambda x : 5, + extract_time_from_instrument_func=lambda _, __ : 5, current_time_func=lambda : 10) assert_that(instrument_data['out_of_sync'], equal_to(False)) @@ -208,7 +208,7 @@ def test_that_GIVEN_time_difference_of_five_THEN_set_time_diff_to_five(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} set_time_shift("", instrument_data, time_shift_threshold=17, - extract_time_from_instrument_func=lambda x : 5, + extract_time_from_instrument_func=lambda _, __ : 5, current_time_func=lambda : 10) assert_that(instrument_data['time_diff'], equal_to(5)) @@ -217,7 +217,7 @@ def test_that_GIVEN_invalid_time_THEN_set_time_diff_to_None(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} set_time_shift("", instrument_data, time_shift_threshold=17, - extract_time_from_instrument_func=lambda x: 'foo', + extract_time_from_instrument_func=lambda _, __: 'foo', current_time_func=lambda: 10) assert_that(instrument_data['time_diff'], equal_to(None)) @@ -226,7 +226,7 @@ def test_that_GIVEN_invalid_time_THEN_set_out_of_sync_to_false(self): instrument_data = {'inst_pvs': {'TIME_OF_DAY': {'value': 'does not matter'}}} set_time_shift("", instrument_data, time_shift_threshold=17, - extract_time_from_instrument_func=lambda x: 'foo', + extract_time_from_instrument_func=lambda _, __: 'foo', current_time_func=lambda: 10) assert_that(instrument_data['out_of_sync'], equal_to(False))