-
Notifications
You must be signed in to change notification settings - Fork 367
Add httpx_trust_env arg for openai_compatible model #791
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
Add httpx_trust_env arg for openai_compatible model #791
Conversation
WalkthroughA new boolean parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OpenAICompatible
participant httpx.Client
participant OpenAI/AzureOpenAI
User->>OpenAICompatible: Instantiate (httpx_trust_env=True/False)
OpenAICompatible->>OpenAICompatible: Check httpx_trust_env value
alt httpx_trust_env is False
OpenAICompatible->>httpx.Client: Create with trust_env=False
OpenAICompatible->>OpenAI/AzureOpenAI: Pass custom http_client
else httpx_trust_env is True
OpenAICompatible->>OpenAI/AzureOpenAI: Use default http_client
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lmms_eval/models/simple/openai_compatible.py (1)
42-52
: Parameter implementation looks good with minor docstring format suggestion.The new parameter follows coding guidelines with proper type hints, default value for backward compatibility, and comprehensive documentation.
Consider using standard docstring format for consistency:
- """ - :param httpx_trust_env: bool - httpx.Client used by openai-python has trust_env set to True by - default. A False value of this param constructs a httpx.Client with - trust_env set to False. Such a httpx.Client ignores environment - variables (HTTP_PROXY, HTTPS_PROXY, ALL_PROXY) and macOS proxy - server settings. - """ + """Initialize OpenAI compatible model. + + Args: + httpx_trust_env: Controls whether httpx.Client should trust environment + proxy settings. When False, creates a client that ignores environment + variables (HTTP_PROXY, HTTPS_PROXY, ALL_PROXY) and macOS proxy + server settings. Defaults to True for backward compatibility. + """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lmms_eval/models/simple/openai_compatible.py
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py
: Type hints required for all code
Public APIs must have docstrings
Functions must be focused and small
Line length: 88 chars maximum
PEP 8 naming (snake_case for functions/variables)
Class names in PascalCase
Constants in UPPER_SNAKE_CASE
Document with docstrings
Use f-strings for formatting
Use early returns to avoid nested conditions
Use clear variable/function names (prefix handlers with "handle")
Use constants where possible instead of functions
Don't repeat yourself (DRY code)
Prefer functional, immutable approaches when not verbose
Define composing functions before their components
Mark issues in existing code with "TODO:" prefix
Use functional and stateless approaches where they improve clarity
Keep core logic clean and push implementation details to the edges
Ruff: Line length (88 chars), import sorting (I001), unused imports
Strings: use parentheses for line wrapping
Function calls: multi-line with proper indent
Imports: split into multiple lines
Explicit None checks for Optional
Type narrowing for strings
Ruff (Python) runs as pre-commit hook
Break strings with parentheses for line length
Multi-line function calls for line length
Split imports for line length
Add None checks for Optional types
Narrow string types
Document public APIs
Files:
lmms_eval/models/simple/openai_compatible.py
🔇 Additional comments (4)
lmms_eval/models/simple/openai_compatible.py (4)
24-24
: LGTM!The import of
DefaultHttpxClient
is correctly added and necessary for the new functionality.
76-76
: LGTM!The HTTP client creation logic correctly handles both cases:
- When
httpx_trust_env=True
: Uses default behavior (http_client=None)- When
httpx_trust_env=False
: Creates custom client withtrust_env=False
78-80
: LGTM!The HTTP client parameter is correctly passed to both OpenAI and AzureOpenAI constructors, enabling the proxy control functionality for both client types.
76-81
: OpenAI Client http_client usage verifiedConfirmed that the OpenAI Python library supports the
http_client
parameter on bothOpenAI
andAzureOpenAI
constructors, and thatDefaultHttpxClient
is the library-providedhttpx.Client
. Your current implementation aligns with the documented usage—no changes required.
Hi, I think you might also need to run |
@yaojingguo Could you add some comments (similar to your messages in this PR) in the code to make people aware of the reason behind this change? ![]() |
c05731a
to
4a9f516
Compare
In China mainland, people usually use a VPN client to access Google. Such a client usually configures macOS proxy server settings.
openai-python
uses ahttpx.Client
withtrust_env
set toTrue
. Such ahttpx.Client
uses macOS proxy server settings.Add
httpx_trust_env
to allow users ignore proxy server settings.Summary by CodeRabbit