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

AzureOpenAI misc fixes #270

Merged
merged 4 commits into from
Apr 6, 2024
Merged

AzureOpenAI misc fixes #270

merged 4 commits into from
Apr 6, 2024

Conversation

chip-davis
Copy link
Contributor

Hello! This PR is by no means production ready, I mainly wanted to alert you to a couple issues I had with Azure and my (super quick) fixes.

Changes made:

  • The upload.py file was always creating a PGVector store with OpenAIEmbeddings(). In my use case, I am only using Azure. So without an openai key set, the app would not start up. I created a simple function that sees if there is an openai key env variable set and returns a PGVector store with OpenAIEmbeddings. If that was not found, it tries the same with Azure. If that was not found, it raises an exception.
  • In llms.py the AzureChatOpenAI was created with the openai_api_base variable set. As of openai>=1.0.0 this has been changed to azure_endpoint.
  • Also in llms.py during the startup of the app, it would crash due to me not having an OpenAI api key set. I created a simple try / catch block to try and fall back to Azure in that instance.

These changes work for me but as I do not have an openai api key I can not test for conflicts caused by these changes.

@mkorpela
Copy link
Collaborator

mkorpela commented Apr 6, 2024

@chip-davis thanks for the contribution! Azure is an important thing to make work.

@mkorpela mkorpela merged commit 27568b5 into langchain-ai:main Apr 6, 2024
6 checks passed
@chip-davis chip-davis deleted the azure-embeddings branch April 6, 2024 17:56
@chip-davis
Copy link
Contributor Author

@chip-davis thanks for the contribution! Azure is an important thing to make work.

Sure thing! Thanks for merging :)

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.

2 participants