Skip to content

Conversation

@yazdipour
Copy link

@yazdipour yazdipour commented Jun 17, 2025

Provide Azure OpenAI API support for Litellm. #6
This PR might not be useful if the goal is to drop litellm. But for others, they can use this branch if they want azure support.

@emeryberger emeryberger requested a review from Copilot June 17, 2025 15:29
Copy link

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 adds support for Azure OpenAI API integration and updates configuration instructions to support both OpenAI and Azure deployments.

  • Introduces Azure configuration in the assistant's initialization and error handling.
  • Refactors the streaming completion call to include Azure-specific API parameters.
  • Updates the README with clear Azure OpenAI configuration instructions.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/chatdbg/assistant/assistant.py Adds Azure environment variable checks, error messaging for Azure, and updates completion query parameters.
README.md Provides detailed instructions for configuring Azure OpenAI alongside the existing OpenAI setup.
Comments suppressed due to low confidence (1)

src/chatdbg/assistant/assistant.py:47

  • [nitpick] It's not immediately clear why 'using_azure' is set to True only when the model name does not start with 'azure'. Consider adding an inline comment to clarify the decision logic behind this condition.
        if all(k in os.environ for k in ["AZURE_API_KEY", "AZURE_API_BASE", "AZURE_API_VERSION"]):

@emeryberger
Copy link
Member

Please see the above review and also run black on your code so it passes the format checks.

@nicovank
Copy link
Collaborator

Thanks! I did some work moving away from LiteLLM, on pause for now but will complete when I get time. In the meantime this is a good fix. Provided we switch back to the vanilla OpenAI API, can you confirm that Azure will then be supported by just imply setting those environment variables?

I'm not super familiar with Azure, looking into testing this over the next few days.

@yazdipour yazdipour marked this pull request as ready for review June 20, 2025 15:14
@yazdipour
Copy link
Author

yazdipour commented Jun 20, 2025

Should be ready now.
Also I tried it on my usecase and it worked.

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.

3 participants