fix: Use apiKey in loadPrompt - #718
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0afb95d561
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| try: | ||
| conn = _authenticated_api_conn(login_result.api_url, api_key) | ||
| finally: | ||
| client.close() |
There was a problem hiding this comment.
Do not reuse an adapter after closing its owner
When set_http_adapter() has installed a custom adapter and a loader uses a separate credential, both the temporary BraintrustClient and _authenticated_api_conn() mount the same _http_adapter instance. This finally closes the client's session, which closes its mounted adapter before the returned connection makes the prompt/parameters request; adapters that release non-recreatable resources on close() will therefore fail, and later cache eviction can also close the adapter used by global connections. Give the loader connection separate adapter ownership or avoid closing an adapter that remains shared.
Useful? React with 👍 / 👎.
| self._loader_login_cache.set(cache_namespace, lazy_state) | ||
| return lazy_state.get() |
There was a problem hiding this comment.
Close loader states evicted while authentication is pending
When more than 16 distinct credentials authenticate concurrently, inserting a new LazyValue can evict one whose authentication is still running. The removal callback sees get_sync() report an unresolved value and does nothing, but this subsequent get() can then finish creating a connection after the entry is no longer tracked, so no reset or later eviction will close it. Repeated multi-tenant bursts can consequently bypass the intended bound and leak sessions; pending evictions need to arrange cleanup after resolution.
Useful? React with 👍 / 👎.
resolves https://linear.app/braintrustdata/issue/SDK-265/load-promptload-parameters-ignore-an-explicit-api-key-and-share-a
python version of braintrustdata/braintrust-sdk-javascript#2375