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

[FIX] Filter API errors #155

Merged
merged 2 commits into from
Nov 8, 2023
Merged

[FIX] Filter API errors #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75f257c
test: expected error to solve
AzulGarza Nov 8, 2023
4c15783
fix: filter api errors with text body
AzulGarza Nov 8, 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
109 changes: 94 additions & 15 deletions nbs/timegpt.ipynb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@
"from tenacity import (\n",
" retry, stop_after_attempt, \n",
" wait_fixed, stop_after_delay,\n",
" RetryCallState, \n",
" retry_if_exception, \n",
" retry_if_not_exception_type,\n",
" RetryCallState,\n",
")\n",
"\n",
"from nixtlats.client import ApiError, Nixtla, SingleSeriesForecast\n",
Expand Down Expand Up @@ -187,12 +188,20 @@
" def after_retry(retry_state: RetryCallState):\n",
" \"\"\"Called after each retry attempt.\"\"\"\n",
" main_logger.info(f\"Attempt {retry_state.attempt_number} failed...\")\n",
" # we want to retry when:\n",
" # there is no ApiError \n",
" # there is an ApiError with string body\n",
" def is_api_error_with_text_body(exception):\n",
" if isinstance(exception, ApiError):\n",
" if isinstance(exception.body, str):\n",
" return True\n",
" return False\n",
" return retry(\n",
" stop=(stop_after_attempt(self.max_retries) | stop_after_delay(self.max_wait_time)),\n",
" wait=wait_fixed(self.retry_interval),\n",
" reraise=True,\n",
" after=after_retry,\n",
" retry=retry_if_not_exception_type(ApiError),\n",
" retry=retry_if_exception(is_api_error_with_text_body) | retry_if_not_exception_type(ApiError),\n",
" )\n",
"\n",
" def _call_api(self, method, kwargs):\n",
Expand Down Expand Up @@ -1251,9 +1260,83 @@
"metadata": {},
"outputs": [],
"source": [
"#| hide\n",
"from httpx import ReadTimeout"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"#| hide\n",
"def test_retry_behavior(side_effect, max_retries=5, retry_interval=5, max_wait_time=40, should_retry=True, sleep_seconds=5):\n",
" mock_timegpt = TimeGPT(\n",
" token=os.environ['TIMEGPT_TOKEN'], \n",
" max_retries=max_retries, \n",
" retry_interval=retry_interval, \n",
" max_wait_time=max_wait_time,\n",
" )\n",
" init_time = time()\n",
" with patch('nixtlats.client.Nixtla.timegpt_multi_series', side_effect=side_effect):\n",
" test_fail(\n",
" lambda: mock_timegpt.forecast(df=df, h=12, time_col='timestamp', target_col='value'),\n",
" )\n",
" total_mock_time = time() - init_time\n",
" if should_retry:\n",
" approx_expected_time = min((max_retries - 1) * retry_interval, max_wait_time)\n",
" upper_expected_time = min(max_retries * retry_interval, max_wait_time)\n",
" assert total_mock_time >= approx_expected_time, \"It is not retrying as expected\"\n",
" # preprocessing time before the first api call should be less than 60 seconds\n",
" assert total_mock_time - upper_expected_time - (max_retries - 1) * sleep_seconds <= sleep_seconds\n",
" else:\n",
" assert total_mock_time <= max_wait_time "
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"#| hide\n",
"# we want the api to retry in these cases\n",
"def raise_api_error_with_text(*args, **kwargs):\n",
" raise ApiError(\n",
" status_code=503, \n",
" body=\"\"\"\n",
" <html><head>\n",
" <meta http-equiv=\"content-type\" content=\"text/html;charset=utf-8\">\n",
" <title>503 Server Error</title>\n",
" </head>\n",
" <body text=#000000 bgcolor=#ffffff>\n",
" <h1>Error: Server Error</h1>\n",
" <h2>The service you requested is not available at this time.<p>Service error -27.</h2>\n",
" <h2></h2>\n",
" </body></html>\n",
" \"\"\")\n",
"test_retry_behavior(raise_api_error_with_text, retry_interval=1)"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"#| hide\n",
"# we want the api to not retry in these cases\n",
"# here A is assuming that the endpoint responds always\n",
"# with a json\n",
"def raise_api_error_with_json(*args, **kwargs):\n",
" raise ApiError(\n",
" status_code=503, \n",
" body=dict(detail='Please use numbers'),\n",
" )\n",
"test_retry_behavior(raise_api_error_with_json, should_retry=False)"
]
},
{
"cell_type": "code",
"execution_count": null,
Expand All @@ -1262,7 +1345,6 @@
"source": [
"#| hide\n",
"# test resilience of api calls\n",
"sleep_seconds = 5\n",
"\n",
"def raise_read_timeout_error(*args, **kwargs):\n",
" print(f'raising ReadTimeout error after {sleep_seconds} seconds')\n",
Expand All @@ -1274,22 +1356,19 @@
" raise HTTPError(response=dict(status_code=503))\n",
" \n",
"combs = [\n",
" (4, 5, 30),\n",
" (2, 5, 30),\n",
" (10, 1, 5),\n",
"]\n",
"side_effects = [raise_read_timeout_error, raise_http_error]\n",
"\n",
"for (max_retries, retry_interval, max_wait_time), side_effect in product(combs, side_effects):\n",
" mock_timegpt = TimeGPT(token=os.environ['TIMEGPT_TOKEN'], max_retries=max_retries, retry_interval=retry_interval, max_wait_time=max_wait_time)\n",
" init_time = time()\n",
" with patch('nixtlats.client.Nixtla.timegpt_multi_series', side_effect=side_effect):\n",
" test_fail(\n",
" lambda: mock_timegpt.forecast(df=df, h=12, time_col='timestamp', target_col='value'),\n",
" )\n",
" total_mock_time = time() - init_time\n",
" approx_epected_time = min((max_retries - 1) * retry_interval, max_wait_time)\n",
" upper_expected_time = min(max_retries * retry_interval, max_wait_time)\n",
" assert total_mock_time >= approx_epected_time\n",
" assert total_mock_time - upper_expected_time - (max_retries - 1) * sleep_seconds <= sleep_seconds # preprocessing time before the first api call shoulb be less than 60 seconds"
" test_retry_behavior(\n",
" max_retries=max_retries, \n",
" retry_interval=retry_interval, \n",
" max_wait_time=max_wait_time, \n",
" side_effect=side_effect,\n",
" )\n",
" "
]
},
{
Expand Down
15 changes: 13 additions & 2 deletions nixtlats/timegpt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
stop_after_attempt,
wait_fixed,
stop_after_delay,
retry_if_not_exception_type,
RetryCallState,
retry_if_exception,
retry_if_not_exception_type,
)

from .client import ApiError, Nixtla, SingleSeriesForecast
Expand Down Expand Up @@ -124,6 +125,15 @@ def after_retry(retry_state: RetryCallState):
"""Called after each retry attempt."""
main_logger.info(f"Attempt {retry_state.attempt_number} failed...")

# we want to retry when:
# there is no ApiError
# there is an ApiError with string body
def is_api_error_with_text_body(exception):
if isinstance(exception, ApiError):
if isinstance(exception.body, str):
return True
return False

return retry(
stop=(
stop_after_attempt(self.max_retries)
Expand All @@ -132,7 +142,8 @@ def after_retry(retry_state: RetryCallState):
wait=wait_fixed(self.retry_interval),
reraise=True,
after=after_retry,
retry=retry_if_not_exception_type(ApiError),
retry=retry_if_exception(is_api_error_with_text_body)
| retry_if_not_exception_type(ApiError),
)

def _call_api(self, method, kwargs):
Expand Down
Loading