Skip to content

remove api keys from metadata build#21

Merged
mskarlin merged 1 commit intomainfrom
remove-api-key-from-metadata
May 23, 2025
Merged

remove api keys from metadata build#21
mskarlin merged 1 commit intomainfrom
remove-api-key-from-metadata

Conversation

@nadolskit
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors how API keys are supplied by removing them from each model's metadata and centralizing them in the ENV_VARS block.

  • Uncomment loading of OPENAI_API_KEY and ANTHROPIC_API_KEY in ENV_VARS
  • Remove api_key entries from each model's litellm_params
Comments suppressed due to low confidence (2)

src/scripts/deploy.py:55

  • Removing the per-model api_key from litellm_params will prevent each model from receiving credentials unless you explicitly inject them elsewhere. Consider updating the model initialization to merge in the global API key (e.g., add a lookup for ENV_VARS["ANTHROPIC_API_KEY"] or ENV_VARS["OPENAI_API_KEY"]).
"litellm_params": {

src/scripts/deploy.py:19

  • Add an explicit validation step for OPENAI_API_KEY and ANTHROPIC_API_KEY at startup (e.g., check os.getenv and raise a clear error) rather than relying on a raw KeyError when accessing os.environ. This provides more user-friendly diagnostics.
"OPENAI_API_KEY": os.environ["OPENAI_API_KEY"],

@nadolskit nadolskit requested a review from jamesbraza May 23, 2025 19:47
Comment thread src/scripts/deploy.py
"model_name": "anthropic/claude-3-7-sonnet-20250219",
"litellm_params": {
"model": "anthropic/claude-3-7-sonnet-20250219",
"api_key": os.environ["ANTHROPIC_API_KEY"],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Any chance you can add a ValueError special case somewhere if there is an api_key in litellm_params? Just thinking about catching this moving forward

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we can totally do that on the server side of the platform 🫡

@mskarlin mskarlin merged commit 38b6df3 into main May 23, 2025
2 checks passed
@mskarlin mskarlin deleted the remove-api-key-from-metadata branch May 23, 2025 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants