mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-09 00:26:01 +00:00
Bugfix/19481 num retries env var type (#19507)
* Enhance error handling for num_retries in Router class to support string values. Add test case to verify conversion from string to int for deployment num_retries. * Refactor Router class for improved readability by formatting long lines and enhancing exception handling tests for num_retries. Ensure consistent style in test cases for better maintainability. * Update exception handling for num_retries in Router class to suppress mypy warnings. Add type ignore comment for clarity in type conversion from string to int.
This commit is contained in:
+14
-5
@@ -1696,8 +1696,11 @@ class Router:
|
||||
|
||||
litellm_params = deployment.get("litellm_params", {})
|
||||
dep_num_retries = litellm_params.get("num_retries")
|
||||
if dep_num_retries is not None and isinstance(dep_num_retries, int):
|
||||
exception.num_retries = dep_num_retries # type: ignore
|
||||
if dep_num_retries is not None:
|
||||
try:
|
||||
exception.num_retries = int(dep_num_retries) # type: ignore # Handle both int and str
|
||||
except (ValueError, TypeError):
|
||||
pass # Skip if value can't be converted to int
|
||||
|
||||
def _update_kwargs_with_default_litellm_params(
|
||||
self, kwargs: dict, metadata_variable_name: Optional[str] = "metadata"
|
||||
@@ -4692,9 +4695,12 @@ class Router:
|
||||
# get num_retries from retry policy
|
||||
# Use the model_group captured at the start of the function, or get it from metadata
|
||||
# kwargs.get("model") at this point is the deployment model, not the model_group
|
||||
_model_group_for_retry_policy = model_group or _metadata.get("model_group") or kwargs.get("model")
|
||||
_model_group_for_retry_policy = (
|
||||
model_group or _metadata.get("model_group") or kwargs.get("model")
|
||||
)
|
||||
_retry_policy_retries = self.get_num_retries_from_retry_policy(
|
||||
exception=original_exception, model_group=_model_group_for_retry_policy
|
||||
exception=original_exception,
|
||||
model_group=_model_group_for_retry_policy,
|
||||
)
|
||||
if _retry_policy_retries is not None:
|
||||
num_retries = _retry_policy_retries
|
||||
@@ -5879,7 +5885,10 @@ class Router:
|
||||
)
|
||||
# done reading model["litellm_params"]
|
||||
# Check if provider is supported: either in enum or JSON-configured
|
||||
if custom_llm_provider not in litellm.provider_list and not JSONProviderRegistry.exists(custom_llm_provider):
|
||||
if (
|
||||
custom_llm_provider not in litellm.provider_list
|
||||
and not JSONProviderRegistry.exists(custom_llm_provider)
|
||||
):
|
||||
raise Exception(f"Unsupported provider - {custom_llm_provider}")
|
||||
|
||||
#### DEPLOYMENT NAMES INIT ########
|
||||
|
||||
@@ -32,17 +32,17 @@ class TestPerDeploymentNumRetries:
|
||||
)
|
||||
|
||||
deployment = router.model_list[0]
|
||||
|
||||
|
||||
# Create a mock exception without num_retries
|
||||
class MockException(Exception):
|
||||
pass
|
||||
|
||||
|
||||
exc = MockException("test error")
|
||||
assert not hasattr(exc, "num_retries") or exc.num_retries is None
|
||||
|
||||
|
||||
# Call the helper
|
||||
router._set_deployment_num_retries_on_exception(exc, deployment)
|
||||
|
||||
|
||||
# Verify num_retries was set from deployment
|
||||
assert exc.num_retries == 5
|
||||
|
||||
@@ -66,16 +66,16 @@ class TestPerDeploymentNumRetries:
|
||||
)
|
||||
|
||||
deployment = router.model_list[0]
|
||||
|
||||
|
||||
# Create an exception that already has num_retries
|
||||
class MockException(Exception):
|
||||
num_retries = 10 # Already set
|
||||
|
||||
|
||||
exc = MockException("test error")
|
||||
|
||||
|
||||
# Call the helper
|
||||
router._set_deployment_num_retries_on_exception(exc, deployment)
|
||||
|
||||
|
||||
# Verify num_retries was NOT overridden
|
||||
assert exc.num_retries == 10
|
||||
|
||||
@@ -99,15 +99,15 @@ class TestPerDeploymentNumRetries:
|
||||
)
|
||||
|
||||
deployment = router.model_list[0]
|
||||
|
||||
|
||||
class MockException(Exception):
|
||||
pass
|
||||
|
||||
|
||||
exc = MockException("test error")
|
||||
|
||||
|
||||
# Call the helper
|
||||
router._set_deployment_num_retries_on_exception(exc, deployment)
|
||||
|
||||
|
||||
# Verify num_retries was not set (deployment has no num_retries)
|
||||
assert not hasattr(exc, "num_retries") or exc.num_retries is None
|
||||
|
||||
@@ -155,3 +155,36 @@ class TestPerDeploymentNumRetries:
|
||||
kwargs = {}
|
||||
router._update_kwargs_before_fallbacks(model="test-model", kwargs=kwargs)
|
||||
assert kwargs["num_retries"] == 7 # Uses global
|
||||
|
||||
def test_set_deployment_num_retries_with_string_value(self):
|
||||
"""
|
||||
Test that _set_deployment_num_retries_on_exception handles string values
|
||||
from environment variables correctly.
|
||||
GitHub Issue: #19481
|
||||
"""
|
||||
router = Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "test-model",
|
||||
"litellm_params": {
|
||||
"model": "openai/gpt-4",
|
||||
"api_key": "test-key",
|
||||
"num_retries": "6", # String value (as from env var)
|
||||
},
|
||||
},
|
||||
],
|
||||
num_retries=0, # Global setting
|
||||
)
|
||||
|
||||
deployment = router.model_list[0]
|
||||
|
||||
class MockException(Exception):
|
||||
pass
|
||||
|
||||
exc = MockException("test error")
|
||||
|
||||
# Call the helper
|
||||
router._set_deployment_num_retries_on_exception(exc, deployment)
|
||||
|
||||
# Verify num_retries was converted from string to int
|
||||
assert exc.num_retries == 6
|
||||
|
||||
Reference in New Issue
Block a user