-
Notifications
You must be signed in to change notification settings - Fork 26
Default base_url for HTTP Client #403
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds default base URL handling for custom HTTP clients passed to Portkey's sync and async client constructors. When users provide their own HTTP client without a base URL, the client will now inherit the base URL from the Portkey client configuration.
Key Changes:
- Added base URL defaulting logic for custom HTTP clients in both sync and async client constructors
- Ensures custom HTTP clients use Portkey's base URL when no base URL is explicitly set
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.allHeaders = self._build_headers(create_model_instance(Options)) | ||
| if http_client: | ||
| http_client.base_url = ( | ||
| http_client.base_url if http_client.base_url != "" else self.base_url |
Copilot
AI
Nov 14, 2025
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.
The comparison http_client.base_url != \"\" may not correctly handle all cases where base_url is unset. HTTPX's base_url is a URL object, not a string, and when unset it defaults to an empty URL object (which doesn't equal an empty string). Consider using if not http_client.base_url or str(http_client.base_url) == \"\" for a more robust check.
| http_client.base_url if http_client.base_url != "" else self.base_url | |
| http_client.base_url if http_client.base_url and str(http_client.base_url) != "" else self.base_url |
| self.allHeaders = self._build_headers(create_model_instance(Options)) | ||
| if http_client: | ||
| http_client.base_url = ( | ||
| http_client.base_url if http_client.base_url != "" else self.base_url |
Copilot
AI
Nov 14, 2025
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.
The comparison http_client.base_url != \"\" may not correctly handle all cases where base_url is unset. HTTPX's base_url is a URL object, not a string, and when unset it defaults to an empty URL object (which doesn't equal an empty string). Consider using if not http_client.base_url or str(http_client.base_url) == \"\" for a more robust check.
| http_client.base_url if http_client.base_url != "" else self.base_url | |
| http_client.base_url if (http_client.base_url and str(http_client.base_url) != "") else self.base_url |
Title: Default base_url for HTTP Client
Description:
Motivation:
Sid asked for it
Related Issues:
Fixes: #402