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

Update Azure OpenAI fields #722

Merged
merged 1 commit into from
Apr 22, 2024
Merged

Conversation

cloutier
Copy link
Contributor

@cloutier cloutier commented Apr 10, 2024

Description

I noticed Azure OpenAI wasn't working in our jupyterhub after an update, and I traced it to us having upgraded to openai 1.x.

I made the changes recommanded by the microsoft doc and I noticed there was also a regression regarding the env var. Unfortunately langchain uses the same env var for both azure and regular openai (sample code) which means people can't use both azure and openai at the same time with jupyter ai, although I think that is a very niche use case.

I have tested my patch in my org and it works now with these changes.

Related issues

Copy link

welcome bot commented Apr 10, 2024

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@3coins 3coins added the bug Something isn't working label Apr 16, 2024
@3coins
Copy link
Collaborator

3coins commented Apr 16, 2024

Unfortunately langchain uses the same env var for both azure and regular openai (sample code) which means people can't use both azure and openai at the same time with jupyter ai, although I think that is a very niche use case.

@cloutier Thanks for making this update. Do you think this needs a fix upstream on the LangChain side? Is there a related issue opened there?

@cloutier cloutier force-pushed the fix-azure-openai-args branch from e87f2cf to 768a698 Compare April 17, 2024 17:37
@cloutier
Copy link
Contributor Author

I looked deeper into it, and I wasn't quite right, langchain uses both env var: https://github.com/langchain-ai/langchain/blob/f2579096993ae460516a0aae1d3e09f3eb5c1772/libs/partners/openai/langchain_openai/llms/azure.py#L85

I have ammended my commit to not change the env var and added a comment about that, but upstream still has the limitation that you can't use both openai and azure openai. They do have a TODO in the code about it.

@dlqqq
Copy link
Member

dlqqq commented Apr 22, 2024

@cloutier I reviewed the source code you linked, and think I understand the issue. Basically we assume the expected keyword argument to be the same as the environment variable name, i.e. AZURE_OPENAI_API_KEY should be passed as azure_openai_api_key=... to the LangChain provider class. However, for authentication, the Azure provider expects the keyword argument openai_api_key, despite preferring AZURE_OPENAI_API_KEY environment variable.

My question to you is: how are you currently authenticating when using Azure in Jupyter AI? Are you setting the AZURE_OPENAI_API_KEY as an environment variable to the jupyter lab process when starting it? I believe I can fix this issue, but I need to know what is currently working for you first.

Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this issue! Approved. ✅

My thoughts:

  • The fields removed in this PR were deprecated in the openai>=1.0 upgrade in LangChain.
  • The issue about AZURE_OPENAI_API_KEY is an existing issue with Jupyter AI; thank you for bringing this to my attention again.
  • Therefore I'm merging this PR, and the API key issue can be addressed in a follow-up PR. 👍

@dlqqq dlqqq force-pushed the fix-azure-openai-args branch from 768a698 to 2695301 Compare April 22, 2024 22:23
@dlqqq dlqqq changed the title update azure openai arguments Update Azure OpenAI fields Apr 22, 2024
@dlqqq dlqqq merged commit b4fab82 into jupyterlab:main Apr 22, 2024
10 checks passed
Copy link

welcome bot commented Apr 22, 2024

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@dlqqq
Copy link
Member

dlqqq commented Apr 22, 2024

@meeseeksdev please backport to 1.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyter-ai that referenced this pull request Apr 22, 2024
dlqqq pushed a commit that referenced this pull request Apr 22, 2024
Marchlak pushed a commit to Marchlak/jupyter-ai that referenced this pull request Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

As of openai>=1.0.0, Azure endpoints should be specified via the azure_endpoint param not openai_api_base
3 participants