Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 46 additions & 8 deletions foundry_sdk/v2/language_models/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from foundry_sdk._core.config import Config
from foundry_sdk._core.context_and_environment_vars import HOSTNAME_VAR
from foundry_sdk._core.context_and_environment_vars import TOKEN_VAR
from foundry_sdk._core.http_client import HttpClient
from foundry_sdk._core.http_client import HttpClient, AsyncHttpClient


def _get_api_gateway_base_url(*, preview: bool = False) -> str:
Expand Down Expand Up @@ -125,20 +125,58 @@ def get_http_client(*, preview: bool = False, config: Optional[Config] = None) -

Raises:
ValueError: If preview is not set to True.
RuntimeError: If the Foundry API gateway base URL or token is not available in the current context.
RuntimeError: If the Foundry token is not available in the current context.
"""
if not preview:
raise ValueError(
"get_http_client() is in beta. " "Please set the preview parameter to True to use it."
)
token = get_foundry_token(preview=True)

# Merge auth header with any user-provided headers
return HttpClient(config=_create_http_client_config(config))
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_create_http_client_config() only injects auth headers but does not appear to set the Foundry API gateway/base URL (or hostname) when config is None. This likely breaks the stated goal of returning clients 'configured for the current Foundry environment' since the client may not know where to send requests. Consider setting base_url (e.g., using _get_api_gateway_base_url(preview=True)) when constructing a new Config, and when config is provided, preserve its base_url (or fill a default if it’s unset) while still merging headers.

Copilot uses AI. Check for mistakes.


def get_async_http_client(
*, preview: bool = False, config: Optional[Config] = None
) -> AsyncHttpClient:
"""Get an async HTTP client configured for the current Foundry environment.
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring claims a RuntimeError can be raised when the API gateway base URL is unavailable, but get_async_http_client() no longer computes/validates the base URL in this diff. Either restore base URL configuration/validation (so the documented error can occur), or update the docstring to only describe the errors actually raised via token retrieval/config handling.

Copilot uses AI. Check for mistakes.

Args:
preview: Must be set to True to use this beta feature.
config: Optional configuration for the async HTTP client.

Returns:
An AsyncHttpClient instance configured with the Foundry hostname and authentication.

Raises:
ValueError: If preview is not set to True.
RuntimeError: If the Foundry token is not available in the current context.
"""
if not preview:
raise ValueError(
"get_async_http_client() is in beta. "
"Please set the preview parameter to True to use it."
)

return AsyncHttpClient(config=_create_http_client_config(config))


def _create_http_client_config(config: Optional[Config]) -> Config:
Comment on lines +160 to +163
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_create_http_client_config() only injects auth headers but does not appear to set the Foundry API gateway/base URL (or hostname) when config is None. This likely breaks the stated goal of returning clients 'configured for the current Foundry environment' since the client may not know where to send requests. Consider setting base_url (e.g., using _get_api_gateway_base_url(preview=True)) when constructing a new Config, and when config is provided, preserve its base_url (or fill a default if it’s unset) while still merging headers.

Copilot uses AI. Check for mistakes.
"""Create a Config object for the HTTP client, ensuring that the Foundry token is included in the headers.

Args:
config: An optional Config object provided by the user.

Returns:
A Config object with the Foundry token included in the default headers.

Raises:
RuntimeError: If the Foundry token is not available in the current context.
"""
token = get_foundry_token(preview=True)
auth_header = {"Authorization": f"Bearer {token}"}

if config is None:
config = Config(default_headers=auth_header)
return Config(default_headers=auth_header)
else:
merged_headers = {**auth_header, **(config.default_headers or {})}
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The merge order allows caller-provided headers to override Authorization, which contradicts the helper’s stated purpose of ensuring the Foundry token is included and can lead to requests being sent with the wrong credentials. If the SDK should guarantee Foundry auth, reverse the merge order so Authorization from auth_header wins (or explicitly set/overwrite only the Authorization key).

Suggested change
merged_headers = {**auth_header, **(config.default_headers or {})}
merged_headers = {**(config.default_headers or {}), **auth_header}

Copilot uses AI. Check for mistakes.
config = replace(config, default_headers=merged_headers)

return HttpClient(config=config)
return replace(config, default_headers=merged_headers)
Comment on lines +179 to +182
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_create_http_client_config() only injects auth headers but does not appear to set the Foundry API gateway/base URL (or hostname) when config is None. This likely breaks the stated goal of returning clients 'configured for the current Foundry environment' since the client may not know where to send requests. Consider setting base_url (e.g., using _get_api_gateway_base_url(preview=True)) when constructing a new Config, and when config is provided, preserve its base_url (or fill a default if it’s unset) while still merging headers.

Copilot uses AI. Check for mistakes.