Skip to content
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

openai[patch]: instantiate clients lazily #29926

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

ccurme
Copy link
Collaborator

@ccurme ccurme commented Feb 21, 2025

#29925

Simplified version of the change below.

Before:

from typing import Any, Optional

import openai
from pydantic import BaseModel, Field, SecretStr, model_validator
from typing_extensions import Self


class BaseChatOpenAI(BaseModel):
    client: Any = Field(default=None, exclude=True)
    async_client: Any = Field(default=None, exclude=True)
    my_special_api_key: Optional[SecretStr] = Field(alias="api_key")

    @model_validator(mode="after")
    def validate_environment(self) -> Self:
        client_params = {"api_key": self.my_special_api_key}
        if not self.client:
            self.client = openai.OpenAI(**client_params)  # type: ignore[arg-type]
        if not self.async_client:
            self.async_client = openai.AsyncOpenAI(**client_params)  # type: ignore[arg-type]
        return self

After:

from typing import Any, Dict, Optional

import openai
from pydantic import BaseModel, Field, PrivateAttr, SecretStr, model_validator
from typing_extensions import Self

class BaseChatOpenAI(BaseModel):
    _client: Any = PrivateAttr(default=None)
    _async_client: Any = PrivateAttr(default=None)
    _client_params: Dict[str, Any] = PrivateAttr(default_factory=dict)
    my_special_api_key: Optional[SecretStr] = Field(alias="api_key")

    # Needed to support initializing the class with `client`
    def __init__(
        self, client: Any = None, async_client: Any = None, **kwargs: Any
    ) -> None:
        super().__init__(**kwargs)
        self._client = client
        self._async_client = async_client

    @model_validator(mode="after")
    def validate_environment(self) -> Self:
        self._client_params = {"api_key": self.my_special_api_key}
        return self

    @property
    def client(self) -> Any:
        if self._client is None:
            self._client = openai.OpenAI(**self._client_params)  # type: ignore[arg-type]
        return self._client

    @client.setter
    def client(self, value: Any) -> None:
        self._client = value

    @property
    def async_client(self) -> Any:
        if self._async_client is None:
            self._async_client = openai.AsyncOpenAI(**self._client_params)  # type: ignore[arg-type]
        return self._async_client

    @async_client.setter
    def async_client(self, value: Any) -> None:
        self._async_client = value

I'm not sure there is a way to do this that is not semver-breaking. One usage pattern I know breaks is subclasses of BaseChatOpenAI, most of which have unique names for their API keys and other attributes (e.g., xai_api_key, together_api_key, etc.). These subclasses have post init steps that look like:

@model_validator(mode="after")
def validate_environment(self) -> Self:
    client_params = {"api_key": self.xai_api_key.get_secret_value()}
    if not self.client:  # <-- breaks
        self.client = openai.OpenAI(**client_params)

The check if not self.client breaks because we now try to instantiate the client in a property, and this requires that client params be accessible to the property.

Current plan is to update classes we control and explain how to update sub-classes in release notes. The update is simple but would be annoying.

Copy link

vercel bot commented Feb 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 22, 2025 3:32pm

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Feb 21, 2025
@ccurme ccurme changed the title openai[patch]: cache clients openai[patch]: instantiate clients lazily Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: Triage
Development

Successfully merging this pull request may close these issues.

1 participant