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

chore(wren-ai-service): separate the api key for each LLM and embedder #917

Merged
merged 2 commits into from
Nov 15, 2024

Conversation

paopa
Copy link
Member

@paopa paopa commented Nov 15, 2024

This PR introduces a clear separation between API keys used for LLM and Embedder services.

Changes

  • Split OPENAI_API_KEY into LLM_OPENAI_API_KEY and EMBEDDER_OPENAI_API_KEY
  • Split AZURE_OPENAI_API_KEY into LLM_AZURE_OPENAI_API_KEY and EMBEDDER_AZURE_OPENAI_API_KEY
  • Updated all relevant configuration files and deployment templates
  • Modified provider initialization to use specific API keys

Implementation Details

Environment Variables

- OPENAI_API_KEY=
- AZURE_OPENAI_API_KEY=
+ LLM_OPENAI_API_KEY=
+ EMBEDDER_OPENAI_API_KEY=
+ LLM_AZURE_OPENAI_API_KEY=
+ EMBEDDER_AZURE_OPENAI_API_KEY=

Provider Updates

  • Updated OpenAI LLM provider to use LLM_OPENAI_API_KEY
  • Updated OpenAI Embedder provider to use EMBEDDER_OPENAI_API_KEY
  • Updated Azure OpenAI providers to use their respective keys
  • Removed fallback to generic API keys

Deployment Changes

  • Updated Kubernetes deployment configurations
  • Modified Docker Compose files to use new environment variables
  • Updated example configuration files and documentation
  • Modified Wren launcher to handle separate API keys

@paopa paopa added module/ai-service ai-service related ci/ai-service ai-service related labels Nov 15, 2024
@paopa paopa marked this pull request as ready for review November 15, 2024 12:20
@paopa paopa changed the title chore(wren-ai-service): split api key for llm and embedder chore(wren-ai-service): separate the api key for each LLM and embedder Nov 15, 2024
@paopa
Copy link
Member Author

paopa commented Nov 15, 2024

@cyyeh I've tried to leave an empty key when langfuse enabling. langfuse client disables itself when key is none or empty string. thus i think we can keep the setting default as true.

here is the source code of langfuse

        if not self.enabled:
            self.log.warning(
                "Langfuse client is disabled. No observability data will be sent."
            )

        elif not public_key:
            self.enabled = False
            self.log.warning(
                "Langfuse client is disabled since no public_key was provided as a parameter or environment variable 'LANGFUSE_PUBLIC_KEY'. See our docs: https://langfuse.com/docs/sdk/python/low-level-sdk#initialize-client"
            )

        elif not secret_key:
            self.enabled = False
            self.log.warning(
                "Langfuse client is disabled since no secret_key was provided as a parameter or environment variable 'LANGFUSE_SECRET_KEY'. See our docs: https://langfuse.com/docs/sdk/python/low-level-sdk#initialize-client"
            )

@paopa paopa requested a review from cyyeh November 15, 2024 12:28
Copy link
Member

@cyyeh cyyeh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot!

@cyyeh cyyeh merged commit 75910d2 into main Nov 15, 2024
11 of 12 checks passed
@cyyeh cyyeh deleted the chore/split-llm-embedder-key branch November 15, 2024 12:45
@paopa
Copy link
Member Author

paopa commented Nov 15, 2024

Migration Guide for Developers Using Docker Compose

After merging PR #792 and this one, follow these steps to update your configuration:

  1. Copy the Configuration Example:

  2. Rename the Configuration File:

    • Rename the copied file to config.yaml.

For more detailed information, please refer to the How to Start with OpenAI section in the Docker README.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/ai-service ai-service related module/ai-service ai-service related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants