azure-openai[patch]: feat: use azureOpenAIApiDeploymentName for all azure-openai #4714
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dosubot
bot
added
size:M
![dosubot[bot]](https://avatars.githubusercontent.com/in/324583?s=60&v=4){kind=small}
| @@ -255,8 +255,8 @@ export class AzureChatOpenAI | |||
There was a problem hiding this comment.
Hey there! I noticed that the recent change in the chat_models.ts file explicitly adds, accesses, reads, or requires an environment variable via process.env or getEnvironmentVariable. I've flagged this for your review to ensure it aligns with our environment variable handling practices. Keep up the great work!
| @@ -48,11 +48,9 @@ export class AzureOpenAIEmbeddings | |||
There was a problem hiding this comment.
Hey there! I've flagged this PR for your review because it includes a change that explicitly accesses an environment variable using the getEnvironmentVariable function. Please take a look and ensure it aligns with our environment variable usage guidelines. Let me know if you have any questions!
| getEnvironmentVariable("AZURE_OPENAI_API_DEPLOYMENT_NAME")); | ||
| this.azureOpenAIApiDeploymentName = | ||
| fieldsWithDefaults?.azureOpenAIApiDeploymentName ?? | ||
| getEnvironmentVariable("AZURE_OPENAI_API_DEPLOYMENT_NAME"); |
There was a problem hiding this comment.
I actually think we want to keep this env var for embeddings and have it use the current behavior - it's apparently common for completions vs. embeddings endpoints.
Yes let's make the change on the input field name though.
jacoblee93
changed the title
|
Thank you @SpringMT! CC @sarangan12 @sinedied this is technically breaking but given the naming is very confusing, I think it'd be good to do now. However, looks like ~6k downloads a week, which is more than 0: https://www.npmjs.com/package/@langchain/azure-openai What do you think? |
|
Discussed with some MS folks, I think the preference would be to avoid a breaking change, even though it's early. I can patch this up later if you'd like. |
|
Thanks for your offer. |
…feature/use-azureOpenAIApiDeploymentName-for-azure-openai
|
Updated to restore backwards compatibility 👍 |
jacoblee93
changed the title
jacoblee93
added
lgtm
|
Thank you! |
SpringMT
deleted the
feature/use-azureOpenAIApiDeploymentName-for-azure-openai
branch
Fixes # (issue)
Ref #4668
In
azure-openai,azureOpenAiembeddingsApiDeploymentNameis deprecated in favor ofazureOpenAIApiDeploymentName.This change introduces consistency but at the cost of losing backward compatibility for using
azureOpenAIEmbeddingsApiDeploymentNameinAzureChatOpenAI.