Skip to content

feat(llm): add public api_key property for in-place credential rotation - #2300

Open
waitasecant wants to merge 1 commit into
NVIDIA-NeMo:developfrom
waitasecant:feat/llm-client
Open

feat(llm): add public api_key property for in-place credential rotation#2300
waitasecant wants to merge 1 commit into
NVIDIA-NeMo:developfrom
waitasecant:feat/llm-client

Conversation

@waitasecant

Copy link
Copy Markdown

Description

Applications that build one long-lived LLMRails/OpenAIChatModel instance at process startup (a common pattern for services that don't want per-request construction overhead) and authenticate against their LLM provider with a short-lived, rotating bearer token (OAuth client-credentials, AWS STS-style tokens, internal gateway tokens, etc.) currently have no supported way to update that credential in place.

Today the only two options are:

  • Reach into private attributes two levels deep (rails.llm._client._api_key), which is unsupported and can silently break on any minor/patch upgrade with no type or import error — just silent 401s from the provider.
  • Rebuild the model via DefaultFramework.create_model() and swap it in with LLMRails.update_llm(). This "looks" like the supported path, but DefaultFramework._clients caches OpenAICompatibleClient instances keyed by (base_url, api_key, ...) and never evicts entries, so every token rotation leaks a new httpx.AsyncClient connection pool for the life of the process.

This PR adds a small, purely additive api_key property (with setter) so callers have a public, documented way to rotate the credential on an already-built model without either workaround.

Related Issue(s)

Verification

Ran pytest and pre-commit checks successfully.

AI Assistance

  • No AI tools were used.
  • AI tools were used; a human reviewed and can explain every change (tool: GitHub Copilot / Claude).

Checklist

  • I've read the CONTRIBUTING guidelines.
  • This PR links to a triaged issue assigned to me.
  • My PR title follows the project commit convention.
  • I've updated the documentation if applicable.
  • I've added tests if applicable.
  • I've noted any verification beyond CI and any checks I couldn't run.
  • I did not update generated changelog files manually.
  • I addressed all CodeRabbit, Greptile, and other review comments, or replied with why no change is needed.
  • @mentions of the person or team responsible for reviewing proposed changes.

@github-actions github-actions Bot added size: M status: needs triage New issues that have not yet been reviewed or categorized. needs: signing labels Aug 18, 2026
Callers that hold one long-lived LLMRails/OpenAIChatModel instance and
authenticate with a rotating bearer token had no supported way to
update it in place, short of reaching into private attributes or
leaking a client via DefaultFramework's api_key-keyed cache.

- Add BaseClient.api_key getter/setter (llm/clients/base.py).
- Delegate through OpenAIChatModel.api_key (llm/models/openai_chat.py).
- Forward through InstrumentedLLMModel.api_key via getattr/setattr so
  tracing/metrics wrapping doesn't silently no-op rotation
  (llm/models/instrumented.py).
- Document api_key as an optional, bearer-token-only extension on the
  LLMModel protocol, not a required member (types.py).
- Add tests covering getter/setter round-trip, header propagation, and
  delegation through the instrumentation decorator.

Purely additive; no existing signature or behavior changes.

Signed-off-by: Himanshu waitasecant@gmail.com
@waitasecant

Copy link
Copy Markdown
Author

Hi @Pouyanpi @tgasser-nv @miyoungc

could you look into this PR put out an update

Friendly ping on this PR 🙂

This is a small, additive change that exposes a public api_key property to support in-place credential rotation without relying on private attributes or recreating/caching new clients on every token refresh.

The PR has been open for about 3 weeks and appears to still be awaiting triage/review. When you have a chance, could you please take a look and/or approve the pending workflows so CI can run?

Thanks for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: M status: needs triage New issues that have not yet been reviewed or categorized.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature: public api_key property for in-place LLM credential rotation

1 participant