review feedback + remove callable api_key from module client

Author: kristapratico

src/openai/__init__.py

@@ -117,7 +117,7 @@
 
 from ._base_client import DEFAULT_TIMEOUT, DEFAULT_MAX_RETRIES
 
-api_key: str | _t.Callable[[], str] | None = None
+api_key: str | None = None
 
 organization: str | None = None
 
@@ -156,27 +156,15 @@ class _ModuleClient(OpenAI):
 
     @property  # type: ignore
     @override
-    def api_key(self) -> str | _t.Callable[[], str] | None:
-        return api_key() if callable(api_key) else api_key
+    def api_key(self) -> str | None:
+        return api_key
 
     @api_key.setter  # type: ignore
-    def api_key(self, value: str | _t.Callable[[], str] | None) -> None:  # type: ignore
+    def api_key(self, value: str | None) -> None:  # type: ignore
         global api_key
         api_key = value
 
-    @property
-    def _api_key_provider(self) -> _t.Callable[[], str] | None: # type: ignore
-        return None
-
-    @_api_key_provider.setter
-    def _api_key_provider(self, value: _t.Callable[[], str] | None) -> None: # type: ignore
-        global api_key
-        # Yes, setting the api_key is intentional. The module level client accepts callables
-        # for the module level api_key and will call it to retrieve the value
-        # if it is a callable.
-        api_key = value
-
-    @property
+    @property  # type: ignore
     @override
     def organization(self) -> str | None:
         return organization

src/openai/_client.py

@@ -8,8 +8,6 @@
 
 import httpx
 
-from openai._models import FinalRequestOptions
-
 from . import _exceptions
 from ._qs import Querystring
 from ._types import (
@@ -27,6 +25,7 @@
     get_async_library,
 )
 from ._compat import cached_property
+from ._models import FinalRequestOptions
 from ._version import __version__
 from ._streaming import Stream as Stream, AsyncStream as AsyncStream
 from ._exceptions import OpenAIError, APIStatusError
@@ -138,7 +137,7 @@ def __init__(
             self.api_key = ""
             self._api_key_provider: Callable[[], str] | None = api_key
         else:
-            self.api_key = api_key or ""
+            self.api_key = api_key
             self._api_key_provider = None
 
         if organization is None:
@@ -477,7 +476,7 @@ def __init__(
             self.api_key = ""
             self._api_key_provider: Callable[[], Awaitable[str]] | None = api_key
         else:
-            self.api_key = api_key or ""
+            self.api_key = api_key
             self._api_key_provider = None
 
         if organization is None:

src/openai/lib/azure.py

@@ -94,7 +94,7 @@ def __init__(
         azure_endpoint: str,
         azure_deployment: str | None = None,
         api_version: str | None = None,
-        api_key: str | None = None,
+        api_key: str | Callable[[], str] | None = None,
         azure_ad_token: str | None = None,
         azure_ad_token_provider: AzureADTokenProvider | None = None,
         organization: str | None = None,
@@ -114,7 +114,7 @@ def __init__(
         *,
         azure_deployment: str | None = None,
         api_version: str | None = None,
-        api_key: str | None = None,
+        api_key: str | Callable[[], str] | None = None,
         azure_ad_token: str | None = None,
         azure_ad_token_provider: AzureADTokenProvider | None = None,
         organization: str | None = None,
@@ -134,7 +134,7 @@ def __init__(
         *,
         base_url: str,
         api_version: str | None = None,
-        api_key: str | None = None,
+        api_key: str | Callable[[], str] | None = None,
         azure_ad_token: str | None = None,
         azure_ad_token_provider: AzureADTokenProvider | None = None,
         organization: str | None = None,
@@ -154,7 +154,7 @@ def __init__(
         api_version: str | None = None,
         azure_endpoint: str | None = None,
         azure_deployment: str | None = None,
-        api_key: str | None = None,
+        api_key: str | Callable[[], str] | None = None,
         azure_ad_token: str | None = None,
         azure_ad_token_provider: AzureADTokenProvider | None = None,
         organization: str | None = None,
@@ -345,7 +345,7 @@ def _configure_realtime(self, model: str, extra_query: Query) -> tuple[httpx.URL
             "api-version": self._api_version,
             "deployment": self._azure_deployment or model,
         }
-        if self.api_key != "<missing API key>":
+        if self.api_key and self.api_key != "<missing API key>":
             auth_headers = {"api-key": self.api_key}
         else:
             token = self._get_azure_ad_token()
@@ -372,7 +372,7 @@ def __init__(
         azure_endpoint: str,
         azure_deployment: str | None = None,
         api_version: str | None = None,
-        api_key: str | None = None,
+        api_key: str | Callable[[], Awaitable[str]] | None = None,
         azure_ad_token: str | None = None,
         azure_ad_token_provider: AsyncAzureADTokenProvider | None = None,
         organization: str | None = None,
@@ -393,7 +393,7 @@ def __init__(
         *,
         azure_deployment: str | None = None,
         api_version: str | None = None,
-        api_key: str | None = None,
+        api_key: str | Callable[[], Awaitable[str]] | None = None,
         azure_ad_token: str | None = None,
         azure_ad_token_provider: AsyncAzureADTokenProvider | None = None,
         organization: str | None = None,
@@ -414,7 +414,7 @@ def __init__(
         *,
         base_url: str,
         api_version: str | None = None,
-        api_key: str | None = None,
+        api_key: str | Callable[[], Awaitable[str]] | None = None,
         azure_ad_token: str | None = None,
         azure_ad_token_provider: AsyncAzureADTokenProvider | None = None,
         organization: str | None = None,

tests/test_client.py

@@ -949,14 +949,13 @@ def test_api_key_before_after_refresh_provider(self) -> None:
         client = OpenAI(base_url=base_url, api_key=lambda: "test_bearer_token")
 
         assert client.api_key == ""
-        assert 'Authorization' not in client.auth_headers
+        assert "Authorization" not in client.auth_headers
 
         client._refresh_api_key()
 
         assert client.api_key == "test_bearer_token"
         assert client.auth_headers.get("Authorization") == "Bearer test_bearer_token"
 
-
     def test_api_key_before_after_refresh_str(self) -> None:
         client = OpenAI(base_url=base_url, api_key="test_api_key")
 
@@ -1956,18 +1955,17 @@ async def test_follow_redirects_disabled(self, respx_mock: MockRouter) -> None:
     async def test_api_key_before_after_refresh_provider(self) -> None:
         async def mock_api_key_provider():
             return "test_bearer_token"
-        
+
         client = AsyncOpenAI(base_url=base_url, api_key=mock_api_key_provider)
 
         assert client.api_key == ""
-        assert 'Authorization' not in client.auth_headers
+        assert "Authorization" not in client.auth_headers
 
         await client._refresh_api_key()
 
         assert client.api_key == "test_bearer_token"
         assert client.auth_headers.get("Authorization") == "Bearer test_bearer_token"
 
-
     @pytest.mark.asyncio
     async def test_api_key_before_after_refresh_str(self) -> None:
         client = AsyncOpenAI(base_url=base_url, api_key="test_api_key")

tests/test_module_client.py

@@ -97,23 +97,6 @@ def test_http_client_option() -> None:
     assert openai.completions._client._client is new_client
 
 
-def test_api_key_callable() -> None:
-    openai.api_key = lambda: "1"
-    assert openai.completions._client.api_key == "1"
-
-def test_api_key_overridable() -> None:
-    openai.api_key = lambda: "1"
-    assert openai.completions._client.api_key == "1"
-    assert openai.completions._client._api_key_provider is None
-
-    openai.api_key = "2"
-    assert openai.completions._client.api_key == "2"
-    assert openai.completions._client._api_key_provider is None
-
-    openai.api_key = lambda: "3"
-    assert openai.completions._client.api_key == "3"
-    assert openai.completions._client._api_key_provider is None
-
 import contextlib
 from typing import Iterator
 
@@ -140,24 +123,6 @@ def test_only_api_key_results_in_openai_api() -> None:
         assert type(openai.completions._client).__name__ == "_ModuleClient"
 
 
-def test_only_api_key_in_openai_api() -> None:
-    with fresh_env():
-        openai.api_type = None
-        openai.api_key = lambda: "example bearer token"
-
-        assert type(openai.completions._client).__name__ == "_ModuleClient"
-
-
-def test_both_api_key_and_api_key_provider_in_openai_api() -> None:
-    with fresh_env():
-        openai.api_key = lambda: "example bearer token"
-
-        assert openai.api_key() == "example bearer token"
-
-        openai.api_key = "example API key"
-        assert openai.api_key == "example API key"
-
-
 def test_azure_api_key_env_without_api_version() -> None:
     with fresh_env():
         openai.api_type = None