-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: Allow OpenAI client config in OpenAIChatGenerator
and AzureOpenAIChatGenerator
#9215
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.
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
Changes from 7 commits
9d3ac7c
5bf23a0
f9853de
b87d92c
b416603
5b455f4
dc5acd9
327e424
fecaab1
841d623
02dcbce
de441ee
17478e5
265a862
4cb3c5b
6ceca1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
# SPDX-FileCopyrightText: 2022-present deepset GmbH <info@deepset.ai> | ||
# | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
from typing import Any, Dict, Optional | ||
|
||
import httpx | ||
|
||
|
||
def init_http_client(http_client_kwargs: Optional[Dict[str, Any]] = None, async_client: bool = False): | ||
""" | ||
Initialize an httpx client based on the http_client_kwargs. | ||
:param http_client_kwargs: | ||
The kwargs to pass to the httpx client. | ||
:param async_client: | ||
Whether to initialize an async client. | ||
:returns: | ||
An httpx client. | ||
""" | ||
if not http_client_kwargs: | ||
return None | ||
if not isinstance(http_client_kwargs, dict): | ||
raise TypeError("The parameter 'http_client_kwargs' must be a dictionary.") | ||
if async_client: | ||
return httpx.AsyncClient(**http_client_kwargs) | ||
Amnah199 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return httpx.Client(**http_client_kwargs) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My 2 cents. In case the user wants to provide their own client, I would leave them free to choose the configurations. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @anakin87 As per my understanding, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I understand this point, as a user I would find it unexpected to pass some client kwargs and then get a client that automatically sets others as well. If we decide to go this path, I would clearly indicate this in all docstrings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alexengrig I see the merits in both approaches. However, we've decided to stick with |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
--- | ||
features: | ||
- | | ||
`OpenAIChatGenerator` and `AzureOpenAIChatGenerator` now support custom HTTP client config via `http_client_kwargs`, enabling proxy and SSL setup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Amnah199!
I want to add similar support to other components, and I think they all should have the same description. I suggest adding documentation similar to the following for
:param http_client:
:A dictionary of keyword arguments to configure a custom
httpx.Client
for your use case: proxies, authentication and other advanced functionalities of HTTPX.You can set a proxy with basic authorization using the environment variables:
HTTP_PROXY
andHTTPS_PROXY
,ALL_PROXY
andNO_PROXY
, for exampleHTTP_PROXY=http://user:password@your-proxy.net:8080
.