From 96d5cb9992d1098a5938aa71a68897b32be31e00 Mon Sep 17 00:00:00 2001 From: Tom Aldcroft Date: Sat, 1 Nov 2025 10:10:18 -0400 Subject: [PATCH 1/4] Improve robustness and retry testing of ephem_stk --- cheta/comps/ephem_stk.py | 20 +++++++++++--------- cheta/tests/test_comps.py | 21 ++++++++++++++++++++- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/cheta/comps/ephem_stk.py b/cheta/comps/ephem_stk.py index 63e5abf1..7039b4ae 100644 --- a/cheta/comps/ephem_stk.py +++ b/cheta/comps/ephem_stk.py @@ -20,6 +20,15 @@ logger = logging.getLogger("cheta.fetch") +# Define standard retry_call for this module +retry_call = functools.partial( + retry_call, + delay=1, + tries=3, + backoff=2, + logger=logger, + mangle_alert_words=True, +) EPHEM_STK_RECENT_DIR = "FOT/mission_planning/Backstop/Ephemeris" EPHEM_STK_ARCHIVE_DIR = "FOT/mission_planning/Backstop/Ephemeris/ArchiveMCC" @@ -178,14 +187,7 @@ def read_stk_file_from_occweb(path: str | Path, format_out="stk", **kwargs) -> T Table of ephemeris data. """ logger.info(f"Reading OCCweb STK file {path}") - text = retry_call( - occweb.get_occweb_page, - args=[path], - kwargs={"timeout": 10} | kwargs, - tries=3, - backoff=2, - logger=logger, - ) + text = retry_call(occweb.get_occweb_page, [path], {"timeout": 10} | kwargs) out = parse_stk_file_text(text, format_out=format_out) return out @@ -309,7 +311,7 @@ def get_ephem_stk_paths( # Then try OCCweb for dir_path in [EPHEM_STK_RECENT_DIR, EPHEM_STK_ARCHIVE_DIR]: logger.info(f"Checking OCCweb directory {dir_path}") - files = occweb.get_occweb_dir(dir_path) + files = retry_call(occweb.get_occweb_dir, [dir_path], {"timeout": 5}) files_stk.extend(find_stk_files(files["Name"], dir_path, "occweb", start, stop)) if latest_only and any(f["start"] <= start for f in files_stk): break diff --git a/cheta/tests/test_comps.py b/cheta/tests/test_comps.py index bf4c8a9e..d19f4bee 100644 --- a/cheta/tests/test_comps.py +++ b/cheta/tests/test_comps.py @@ -10,11 +10,13 @@ import numpy as np import pytest from cxotime import CxoTime +from kadi import occweb from Quaternion import Quat +from ska_helpers.retry import MockFuncFailure from .. import fetch as fetch_cxc from .. import fetch_eng, fetch_sci -from ..comps import ComputedMsid +from ..comps import ComputedMsid, ephem_stk from ..comps.ephem_stk import ( EPHEM_STK_DIRS_DEFAULT, get_ephem_stk_paths, @@ -626,3 +628,20 @@ def test_dp_roll_css(): ) # fmt: on assert np.allclose(vals, exp, rtol=0, atol=2e-4) + + +def test_stk_ephem_timeout(monkeypatch, tmp_path, clear_lru_cache): + monkeypatch.setattr( + ephem_stk, "EPHEM_STK_CACHE_DIR_DEFAULT", str(tmp_path / "cache") + ) + mock_get_occ_web_page = MockFuncFailure(occweb.get_occweb_page, n_fail=1) + monkeypatch.setattr(occweb, "get_occweb_page", mock_get_occ_web_page) + fetch_cxc.add_logging_handler() + + result = fetch_cxc.Msid("orbitephem_stk_x", "2023:001", "2023:002") + + assert result.MSID == "ORBITEPHEM_STK_X" + # Three places call get_occweb_page (once via get_occweb_dir). Each of those will + # have at least one fail and one success, so >= 6 calls total. There could be a + # bona-fide network failure giving more than 6 calls. + assert len(mock_get_occ_web_page.calls) >= 6 From 64db99d883005f05d18bb659369a8e83e6ba9ed4 Mon Sep 17 00:00:00 2001 From: Tom Aldcroft Date: Sat, 1 Nov 2025 10:17:05 -0400 Subject: [PATCH 2/4] Remove adding the fetch logging handler (leftover from debug) --- cheta/tests/test_comps.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cheta/tests/test_comps.py b/cheta/tests/test_comps.py index d19f4bee..d2cba555 100644 --- a/cheta/tests/test_comps.py +++ b/cheta/tests/test_comps.py @@ -636,7 +636,6 @@ def test_stk_ephem_timeout(monkeypatch, tmp_path, clear_lru_cache): ) mock_get_occ_web_page = MockFuncFailure(occweb.get_occweb_page, n_fail=1) monkeypatch.setattr(occweb, "get_occweb_page", mock_get_occ_web_page) - fetch_cxc.add_logging_handler() result = fetch_cxc.Msid("orbitephem_stk_x", "2023:001", "2023:002") From af8d5927e2f43281773e52fbb28a59b1b778bfb4 Mon Sep 17 00:00:00 2001 From: Tom Aldcroft Date: Thu, 13 Nov 2025 06:42:16 -0500 Subject: [PATCH 3/4] Update for upstream retry in occweb --- cheta/comps/ephem_stk.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/cheta/comps/ephem_stk.py b/cheta/comps/ephem_stk.py index 7039b4ae..ec0b538c 100644 --- a/cheta/comps/ephem_stk.py +++ b/cheta/comps/ephem_stk.py @@ -14,21 +14,11 @@ from astropy.table import Column, Table from cxotime import CxoTime, CxoTimeLike from kadi import occweb -from ska_helpers.retry import retry_call from .computed_msid import ComputedMsid logger = logging.getLogger("cheta.fetch") -# Define standard retry_call for this module -retry_call = functools.partial( - retry_call, - delay=1, - tries=3, - backoff=2, - logger=logger, - mangle_alert_words=True, -) EPHEM_STK_RECENT_DIR = "FOT/mission_planning/Backstop/Ephemeris" EPHEM_STK_ARCHIVE_DIR = "FOT/mission_planning/Backstop/Ephemeris/ArchiveMCC" @@ -187,7 +177,8 @@ def read_stk_file_from_occweb(path: str | Path, format_out="stk", **kwargs) -> T Table of ephemeris data. """ logger.info(f"Reading OCCweb STK file {path}") - text = retry_call(occweb.get_occweb_page, [path], {"timeout": 10} | kwargs) + kwargs = {"timeout": 10} | kwargs + text = occweb.get_occweb_page(path, **kwargs) out = parse_stk_file_text(text, format_out=format_out) return out @@ -311,7 +302,7 @@ def get_ephem_stk_paths( # Then try OCCweb for dir_path in [EPHEM_STK_RECENT_DIR, EPHEM_STK_ARCHIVE_DIR]: logger.info(f"Checking OCCweb directory {dir_path}") - files = retry_call(occweb.get_occweb_dir, [dir_path], {"timeout": 5}) + files = occweb.get_occweb_dir(dir_path, timeout=5) files_stk.extend(find_stk_files(files["Name"], dir_path, "occweb", start, stop)) if latest_only and any(f["start"] <= start for f in files_stk): break From 2cb5c39aba3a4d77a02722ff7495a97d179710bb Mon Sep 17 00:00:00 2001 From: Tom Aldcroft Date: Thu, 13 Nov 2025 07:17:53 -0500 Subject: [PATCH 4/4] Improve test --- cheta/tests/test_comps.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/cheta/tests/test_comps.py b/cheta/tests/test_comps.py index d2cba555..4ab036b1 100644 --- a/cheta/tests/test_comps.py +++ b/cheta/tests/test_comps.py @@ -9,10 +9,11 @@ import astropy.units as u import numpy as np import pytest +import requests +import ska_helpers.retry.api from cxotime import CxoTime -from kadi import occweb from Quaternion import Quat -from ska_helpers.retry import MockFuncFailure +from ska_helpers import retry from .. import fetch as fetch_cxc from .. import fetch_eng, fetch_sci @@ -634,13 +635,19 @@ def test_stk_ephem_timeout(monkeypatch, tmp_path, clear_lru_cache): monkeypatch.setattr( ephem_stk, "EPHEM_STK_CACHE_DIR_DEFAULT", str(tmp_path / "cache") ) - mock_get_occ_web_page = MockFuncFailure(occweb.get_occweb_page, n_fail=1) - monkeypatch.setattr(occweb, "get_occweb_page", mock_get_occ_web_page) + mock_requests_get = retry.MockFuncFailure(requests.get, n_fail=1) + monkeypatch.setattr(requests, "get", mock_requests_get) + # Make the test run faster by monkeypatching the default retry delay + monkeypatch.setattr( + ska_helpers.retry.api, + "RETRY_DEFAULTS", + ska_helpers.retry.api.RETRY_DEFAULTS | {"delay": 0.01}, + ) result = fetch_cxc.Msid("orbitephem_stk_x", "2023:001", "2023:002") assert result.MSID == "ORBITEPHEM_STK_X" - # Three places call get_occweb_page (once via get_occweb_dir). Each of those will + # Three places call requests.get (once via get_occweb_dir). Each of those will # have at least one fail and one success, so >= 6 calls total. There could be a # bona-fide network failure giving more than 6 calls. - assert len(mock_get_occ_web_page.calls) >= 6 + assert len(mock_requests_get.calls) >= 6