fix(router.py): fix setting httpx mounts #4434
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
||
| # Retrieve NO_PROXY environment variable | ||
| no_proxy = os.getenv("NO_PROXY", None) | ||
| no_proxy_urls = no_proxy.split(",") if no_proxy else [] |
There was a problem hiding this comment.
get_proxy_environment() is already considering the no_proxy values. Those are the values, which have a None assigned to. So you don't need to read here.
| async_proxy_mounts[key] = httpx.AsyncHTTPTransport() | ||
| else: | ||
| sync_proxy_mounts[key] = httpx.HTTPTransport(proxy=proxy) | ||
| async_proxy_mounts[key] = httpx.AsyncHTTPTransport(proxy=proxy) |
There was a problem hiding this comment.
The problem with this is, that you create Transports here, which are different to the Transport you actually want to use later. With this solution, it would use the proxy Transports for all urls, which must use proxy and the no-proxy Transports for the urls which match the NO_PROXY environment variable. Which means, it would never use the transport you assign later with the argument transport to the httpx.AsyncClient.
Instead you have to do it differently:
- create the transport you actually want to use for all connections (for example
AsyncCustomHTTPTransport) - assign this proxy to all urls in
proxies, where the value isNone - for every value different to
None, create a new transport of the type you want to use (for exampleAsyncCustomHTTPTransport) or maybe copy it, and apply the proxy value to it
| os.environ["HTTPS_PROXY"] = "https://proxy.example.com:8080" | ||
| from openai import AsyncAzureOpenAI | ||
|
|
||
| # Function to check if a proxy is set on the client |
litellm/router.py
Outdated
| for url in no_proxy_urls: # set no-proxy support for specific urls | ||
| sync_proxy_mounts[url] = None # type: ignore | ||
| async_proxy_mounts[url] = None # type: ignore | ||
| sync_proxy_mounts, async_proxy_mounts = create_proxy_transport_and_mounts() |
There was a problem hiding this comment.
The same logic must also be fixed on other places:
- litellm\llms\custom_httpx\http_handler.py in
create_clientof classAsyncHTTPHandler - litellm\llms\custom_httpx\http_handler.py in
__init__of classHTTPHandler
Not needed after azure dall-e-2 refactor
There was a problem hiding this comment.
Looks very good now. I just noticed a inconsistency in one of the test-cases, which can cause issues on systems, which have set HTTP_PROXY, HTTPS_PROXY and NO_PROXY in their environment.
Just a question out of curiosity: Why is the custom async httpx client not needed anymore? I was thinking it is needed for dall-e?
| @pytest.mark.asyncio | ||
| async def test_is_proxy_set(): | ||
| """ | ||
| Assert if proxy is set | ||
| """ | ||
| from httpcore import AsyncHTTPProxy | ||
| from httpx import AsyncHTTPTransport | ||
|
|
||
| os.environ["HTTPS_PROXY"] = "https://proxy.example.com:8080" |
There was a problem hiding this comment.
I think this could be an issue, because it sets/overwrites the proxy settings from the environment and does not reset those.
I would suggest using a setup/teardown function to store/clean and restore the current environment settings. Or to use a fixture for that.
What you basically have to do is:
- read the current values of
HTTPS_PROXY,HTTP_PROXYandNO_PROXYinto a variable - clean all those values and only set
HTTP_PROXYas you do in this line here - after the test (even if it fails), reset
HTTPS_PROXY,HTTP_PROXYandNO_PROXYto the values, you have read and stored in point 1
There was a problem hiding this comment.
I found still a bug by using no_proxy on other places.
Furthermore I get the error:
Traceback (most recent call last):
... (deleted our code here) ...
File "/llm_api/.venv/lib/python3.12/site-packages/litellm/__init__.py", line 743, in <module>
from litellm.litellm_core_utils.litellm_logging import Logging
File "/llm_api/.venv/lib/python3.12/site-packages/litellm/litellm_core_utils/litellm_logging.py", line 62, in <module>
from ..integrations.langfuse import LangFuseLogger
File "/llm_api/.venv/lib/python3.12/site-packages/litellm/integrations/langfuse.py", line 7, in <module>
from packaging.version import Version
ModuleNotFoundError: No module named 'packaging'
Can it be, that packaging is not added to the dependencies in this version?
If I add it to our project dependencies it runs, but the import is inside litellm.
| @@ -3378,40 +3375,6 @@ def set_client(self, model: dict): | |||
| litellm_params["max_retries"] = max_retries | |||
|
|
|||
| # proxy support | |||
| import os | |||
There was a problem hiding this comment.
There is still similar code in litellm\llms\custom_httpx\http_handler.py, which leads to errors, when importing litellm, because it defines mounts by using values from the NO_PROXY. It is in __init__(...) of HTTPHandler and in create_client(...) of AsyncHTTPHandler.
Title
Fix setting httpx mounts
Relevant issues
Fixes #4376
Fixes #2127
Fixes #3556
Type
🐛 Bug Fix
🧹 Refactoring
Changes
[REQUIRED] Testing - Attach a screenshot of any new tests passing locall
If UI changes, send a screenshot/GIF of working UI fixes
test_is_proxy_setto check if the client set uses the proxy