Skip to content
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions ols/app/models/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ def __init__(
constants.PROVIDER_RHOAI_VLLM,
constants.PROVIDER_RHELAI_VLLM,
constants.PROVIDER_OPENAI,
constants.PROVIDER_AZURE_OPENAI,
):
self.certificates_store = os.path.join(
certificate_directory, constants.CERTIFICATE_STORAGE_FILENAME
Expand Down
7 changes: 4 additions & 3 deletions ols/src/llms/providers/azure_openai.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def default_params(self) -> dict[str, Any]:
deployment_name = self.provider_config.deployment_name
azure_config = self.provider_config.azure_config

# provider-specific configuration has precendence over regular configuration
# provider-specific configuration has precedence over regular configuration
if azure_config is not None:
self.url = str(azure_config.url)
deployment_name = azure_config.deployment_name
Expand All @@ -73,8 +73,8 @@ def default_params(self) -> dict[str, Any]:
"cache": None,
"max_tokens": 512,
"verbose": False,
"http_client": self._construct_httpx_client(False, False),
"http_async_client": self._construct_httpx_client(False, True),
"http_client": self._construct_httpx_client(True, False),
"http_async_client": self._construct_httpx_client(True, True),
}

# gpt-5 and o-series models don't support certain parameters
Expand All @@ -92,6 +92,7 @@ def default_params(self) -> dict[str, Any]:
# client_id and client_secret)
access_token = self.resolve_access_token(azure_config)
default_parameters["azure_ad_token"] = access_token
logger.info("Created Azure default parameters %s", default_parameters)
return default_parameters

def load(self) -> BaseChatModel:
Expand Down
7 changes: 7 additions & 0 deletions ols/src/llms/providers/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ def _construct_httpx_client( # noqa: C901
}

sec_profile = self.provider_config.tls_security_profile
logger.info("Security profile %s", sec_profile)

# if security profile is not set, use httpx client as is
if sec_profile is None or sec_profile.profile_type is None:
Expand All @@ -385,6 +386,9 @@ def _construct_httpx_client( # noqa: C901
cafile=self.provider_config.certificates_store
)
verify = custom_context
logger.info(
"No security profiles. creating httpx.Client with verify %s", verify
)
if use_async:
return httpx.AsyncClient(verify=verify, proxies=proxy, mounts=mounts)
return httpx.Client(verify=verify, proxies=proxy, mounts=mounts)
Expand All @@ -411,6 +415,9 @@ def _construct_httpx_client( # noqa: C901

if use_custom_certificate_store:
context.load_verify_locations(self.provider_config.certificates_store)
logger.info(
"With security profile, creating httpx.Client with verify %s", context
)
if use_async:
return httpx.AsyncClient(verify=context, proxies=proxy)
return httpx.Client(verify=context, proxies=proxy)
24 changes: 21 additions & 3 deletions tests/unit/llms/providers/test_azure_openai.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Unit tests for Azure OpenAI provider."""

import os
import time
from unittest.mock import patch

Expand All @@ -9,13 +10,28 @@
from langchain_openai import AzureChatOpenAI
from pydantic import AnyHttpUrl

from ols import constants
from ols.app.models.config import AzureOpenAIConfig, ProviderConfig
from ols.src.llms.providers.azure_openai import (
TOKEN_EXPIRATION_LEEWAY,
AzureOpenAI,
TokenCache,
)

cert_in_certificates_store_path = "tests/unit/extra_certs/sample_cert_1.crt"


@pytest.fixture
def fake_certifi_store(tmpdir):
"""Create a fake certifi store."""
cert_store_path = os.path.join(
constants.DEFAULT_CERTIFICATE_DIRECTORY, constants.CERTIFICATE_STORAGE_FILENAME
)
with open(cert_store_path, "wb") as cert_store:
with open(cert_in_certificates_store_path, "rb") as cert_file:
cert_store.write(cert_file.read())
return cert_store_path

Copy link
Contributor

Choose a reason for hiding this comment

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

@blublinsky What is the use of this fixture , the tests dont use them - Am I missing anything here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, two methods are using it as a parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was the reason why unit tests were failing

Copy link
Contributor

Choose a reason for hiding this comment

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

The same fixture is defined in other providers that support custom certs.

This fixture stores the cert permanently into the location. Which is wrong - unit test shouldn't leave artifacts.
But it is out of the scope of this PR to fix it (probably not worth it, given the lcore migration anyway).


@pytest.fixture
def provider_config():
Expand Down Expand Up @@ -189,7 +205,7 @@ def provider_config_access_token_related_parameters():
)


def test_basic_interface(provider_config):
def test_basic_interface(provider_config, fake_certifi_store):
"""Test basic interface."""
azure_openai = AzureOpenAI(
model="uber-model", params={}, provider_config=provider_config
Expand Down Expand Up @@ -236,7 +252,9 @@ def test_credentials_in_directory_handling(provider_config_credentials_directory
assert azure_openai.default_params["api_key"] == "secret_key"


def test_loading_provider_specific_parameters(provider_config_with_specific_parameters):
def test_loading_provider_specific_parameters(
provider_config_with_specific_parameters, fake_certifi_store
):
"""Test if provider-specific parameters are loaded too."""
azure_openai = AzureOpenAI(
model="uber-model",
Expand Down Expand Up @@ -268,7 +286,7 @@ def test_loading_provider_specific_parameters(provider_config_with_specific_para
assert azure_openai.credentials == "secret_key_2"


def test_params_handling(provider_config):
def test_params_handling(provider_config, fake_certifi_store):
"""Test that not allowed parameters are removed before model init."""
# first three parameters should be removed before model init
# rest need to stay
Expand Down