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

Add support for Azure OpenAI in LangChainOpenAIHandler #567

Conversation

zmeir
Copy link
Contributor

@zmeir zmeir commented Jan 4, 2024

Type

Enhancement


Description

This PR introduces the following changes:

  • Adds support for Azure OpenAI in the LangChainOpenAIHandler class.
  • Improves error handling in the LangChainOpenAIHandler class to provide more specific error messages.
  • Removes logging related to the generation of completion with a specific model from the chat_completion method in litellm_ai_handler.py.
  • Adds logging related to the generation of prediction with a specific model to the retry_with_fallback_models function in pr_processing.py.

PR changes walkthrough

Relevant files                                                                                                                                 
Enhancement
1 files
langchain_ai_handler.py                                                                         
    pr_agent/algo/ai_handlers/langchain_ai_handler.py

    The file has been modified to add support for Azure OpenAI
    in the LangChainOpenAIHandler class. This includes changes
    in the initialization of the _chat attribute, where a
    check is made to determine if the OpenAI API type is Azure.
    If it is, the _chat attribute is set to an instance of
    AzureChatOpenAI, otherwise, it remains as an instance of
    ChatOpenAI. Error handling has also been improved to
    provide more specific error messages.

+25/-7
Miscellaneous
2 files
litellm_ai_handler.py                                                                             
    pr_agent/algo/ai_handlers/litellm_ai_handler.py

    The logging related to the generation of completion with a
    specific model has been removed from the chat_completion
    method.

+0/-5
pr_processing.py                                                                                       
    pr_agent/algo/pr_processing.py

    The logging related to the generation of prediction with a
    specific model has been added to the
    retry_with_fallback_models function.

+5/-0

Copy link

PR Description updated to latest commit (2790e59)

Copy link

PR Analysis

  • 🎯 Main theme: Adding support for Azure OpenAI in LangChainOpenAIHandler and improving error handling
  • 📝 PR summary: This PR introduces support for Azure OpenAI in the LangChainOpenAIHandler class. It also improves error handling in the same class to provide more specific error messages. Additionally, it moves logging related to the generation of predictions from the chat_completion method in litellm_ai_handler.py to the retry_with_fallback_models function in pr_processing.py.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: No
  • ⏱️ Estimated effort to review [1-5]: 3, because the PR involves changes in the core functionality of the AI handler and requires a good understanding of the existing codebase and the OpenAI API.
  • 🔒 Security concerns: No security concerns found

PR Feedback

💡 General suggestions: It's great to see the addition of Azure OpenAI support. However, it would be beneficial to add tests to ensure the new functionality works as expected. Also, consider handling the case where neither Azure nor OpenAI is selected in the settings.

🤖 Code feedback:
relevant filepr_agent/algo/ai_handlers/langchain_ai_handler.py
suggestion      

Consider adding a default case in the __init__ method of LangChainOpenAIHandler class to handle the scenario where neither Azure nor OpenAI is selected in the settings. This will prevent potential issues in the future. [important]

relevant lineif self.azure:

relevant filepr_agent/algo/ai_handlers/langchain_ai_handler.py
suggestion      

It would be beneficial to add a comment explaining why the deployment_id is set only in the chat property for Azure and not in the __init__ method. This will improve code readability. [medium]

relevant lineif self.azure:

relevant filepr_agent/algo/ai_handlers/langchain_ai_handler.py
suggestion      

Consider handling the case where the name attribute does not exist in the exception object. Currently, if the name attribute is not present, the original exception is raised, which might not be very informative. [medium]

relevant lineif getattr(e, "name"):

relevant filepr_agent/algo/pr_processing.py
suggestion      

Consider adding a comment explaining why the logging was moved from the chat_completion method in litellm_ai_handler.py to the retry_with_fallback_models function. This will help other developers understand the reasoning behind this change. [medium]

relevant lineif get_settings().config.verbosity_level >= 2:

✨ Usage tips:

To invoke the PR-Agent, add a comment using one of the following commands:

  • /review: Request a review of your Pull Request.
  • /describe: Update the PR title and description based on the contents of the PR.
  • /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
  • /ask <QUESTION>: Ask a question about the PR.
  • /update_changelog: Update the changelog based on the PR's contents.
  • /add_docs 💎: Generate docstring for new components introduced in the PR.
  • /generate_labels 💎: Generate labels for the PR based on the PR's contents.
  • /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component.

See the tools guide for more details.
To edit any configuration parameter from the configuration.toml, add --config_path=new_value.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, add a /config comment.

@zmeir zmeir force-pushed the zmeir/enhance/support_azure_in_langchain_ai_handler branch from 2790e59 to ba3f22d Compare January 4, 2024 14:22
@mrT23
Copy link
Collaborator

mrT23 commented Jan 4, 2024

Looks good. now there is an actual use case for langChain handelr :-)

@mrT23 mrT23 merged commit e6093cd into Codium-ai:main Jan 4, 2024
@zmeir zmeir deleted the zmeir/enhance/support_azure_in_langchain_ai_handler branch January 4, 2024 16:35
yochail pushed a commit to yochail/pr-agent that referenced this pull request Feb 11, 2024
…re_in_langchain_ai_handler

Add support for Azure OpenAI in LangChainOpenAIHandler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants