Skip to content

Commit a112bc3

Browse files
committed
feat: enhance upstream fetch with retry logic for rate limits and non-retryable errors
1 parent 7d2373b commit a112bc3

2 files changed

Lines changed: 62 additions & 3 deletions

File tree

application/cmd/cre_main.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,22 @@ def fetch_upstream_json(
6969
last_error = exc
7070

7171
if attempt < max_attempts:
72+
retry_after_header = None
73+
sleep_seconds = backoff_seconds * attempt
74+
if "response" in locals() and response.status_code == 429:
75+
retry_after_header = response.headers.get("Retry-After")
76+
try:
77+
if retry_after_header is not None:
78+
sleep_seconds = float(retry_after_header)
79+
except (TypeError, ValueError):
80+
sleep_seconds = backoff_seconds * attempt
7281
logger.warning(
7382
"upstream fetch failed for %s on attempt %s/%s, retrying",
7483
url,
7584
attempt,
7685
max_attempts,
7786
)
78-
time.sleep(backoff_seconds * attempt)
87+
time.sleep(sleep_seconds)
7988

8089
if last_error:
8190
raise RuntimeError(f"upstream fetch failed for {url}") from last_error
@@ -635,11 +644,11 @@ def download_graph_from_upstream(cache: str) -> None:
635644
collection = db_connect(path=cache).with_graph()
636645

637646
def download_cre_from_upstream(creid: str):
647+
if creid in imported_cres:
648+
return
638649
data = fetch_upstream_json(f"/id/{creid}")
639650
credict = data["data"]
640651
cre = defs.Document.from_dict(credict)
641-
if cre.id in imported_cres:
642-
return
643652

644653
register_cre(cre, collection)
645654
imported_cres[cre.id] = ""

application/tests/cre_main_test.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,56 @@ def test_fetch_upstream_json_retries_transient_failures(
488488
self.assertEqual(mock_get.call_count, 2)
489489
mock_sleep.assert_called_once()
490490

491+
@patch("application.cmd.cre_main.time.sleep")
492+
@patch("application.cmd.cre_main.requests.get")
493+
def test_fetch_upstream_json_fails_fast_on_non_retryable_status(
494+
self, mock_get, mock_sleep
495+
) -> None:
496+
response = Mock()
497+
response.status_code = 404
498+
response.headers = {}
499+
mock_get.return_value = response
500+
501+
with self.assertRaises(RuntimeError):
502+
main.fetch_upstream_json("/root_cres")
503+
504+
self.assertEqual(mock_get.call_count, 1)
505+
mock_sleep.assert_not_called()
506+
507+
@patch("application.cmd.cre_main.time.sleep")
508+
@patch("application.cmd.cre_main.requests.get")
509+
def test_fetch_upstream_json_retries_retryable_status_until_exhausted(
510+
self, mock_get, mock_sleep
511+
) -> None:
512+
response = Mock()
513+
response.status_code = 503
514+
response.headers = {}
515+
mock_get.return_value = response
516+
517+
with self.assertRaises(RuntimeError):
518+
main.fetch_upstream_json("/root_cres", max_attempts=3, backoff_seconds=2)
519+
520+
self.assertEqual(mock_get.call_count, 3)
521+
self.assertEqual(mock_sleep.call_count, 2)
522+
mock_sleep.assert_any_call(2)
523+
mock_sleep.assert_any_call(4)
524+
525+
@patch("application.cmd.cre_main.time.sleep")
526+
@patch("application.cmd.cre_main.requests.get")
527+
def test_fetch_upstream_json_honors_retry_after_for_rate_limit(
528+
self, mock_get, mock_sleep
529+
) -> None:
530+
response = Mock()
531+
response.status_code = 429
532+
response.headers = {"Retry-After": "7"}
533+
mock_get.side_effect = [response, response, RuntimeError("should not reach")]
534+
535+
with self.assertRaises(RuntimeError):
536+
main.fetch_upstream_json("/root_cres", max_attempts=2, backoff_seconds=2)
537+
538+
self.assertEqual(mock_get.call_count, 2)
539+
mock_sleep.assert_called_once_with(7.0)
540+
491541
@patch.object(main, "db_connect")
492542
@patch.object(Queue, "enqueue_call")
493543
@patch.object(redis, "wait_for_jobs")

0 commit comments

Comments
 (0)