openai[minor]: Update OpenAI with Azure Specific Code #5323

sarangan12 · 2024-05-09T06:37:35Z

Azure SDK Team has made changes to Open AI Node SDK in PR openai/openai-node#718. In that PR, a new AzureOpenAI Client has been introduced. This enables the OpenAI SDK Users to access Azure Hosted Services with their credentials object instead of OpenAI Key.

This PR is to make changes to @langchain/openai package to make the corresponding changes so Langchain users also could access their Azure Hosted OpenAI service with the credentials object. Required test cases have already been added to the PR.

@achandmsft @deyaaeldeen @xirzec FYI.....

vercel · 2024-05-09T06:37:39Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
langchainjs-api-refs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 13, 2024 11:45pm
langchainjs-docs	✅ Ready (Inspect)	Visit Preview		May 13, 2024 11:45pm

…azure

dosubot · 2024-05-09T06:50:38Z

libs/langchain-openai/package.json

@@ -41,11 +41,12 @@
 "dependencies": {


Hey there! 👋 I noticed that the package.json file has an update to the "openai" dependency version and a new dev dependency "@azure/identity" added. This is just a heads up for the maintainers to review the changes in dependencies. Keep up the great work! 🚀

dosubot · 2024-05-09T06:50:39Z

libs/langchain-openai/src/azure/chat_models.ts

@@ -48,6 +43,56 @@ export class AzureChatOpenAI extends ChatOpenAI {
 super(newFields);


Hey there! I've reviewed the code changes and it looks like a new HTTP request setup using the AzureOpenAIClient has been added in the _getClientOptions method. I've flagged this for your review to ensure it aligns with the project's requirements. Let me know if you have any questions or need further clarification.

dosubot · 2024-05-09T06:50:40Z

libs/langchain-openai/src/tests/azure/chat_models.int.test.ts

@@ -0,0 +1,829 @@
+import { test, jest, expect } from "@jest/globals";


Hey team, I've reviewed the code and noticed that the changes in this PR access environment variables using the getEnvironmentVariable function. This comment is to flag the change for your review. Let me know if you need any further assistance!

dosubot · 2024-05-09T06:50:41Z

libs/langchain-openai/src/tests/azure/llms.int.test.ts

@@ -0,0 +1,333 @@
+import { test, expect } from "@jest/globals";


Hey team, just a heads up that I've flagged a change in the PR that uses the getEnvironmentVariable function to access environment variables. It's important for maintainers to review this change to ensure proper handling of environment variables.

bracesproul

Why wouldn't you just use the @langchain/azure-openai package?

sarangan12 · 2024-05-10T17:53:52Z

The @langchain/azure-openai package uses the @azure/openai SDK to access azure. Now, openai package itself has added features to support the same (independent of the @azure/openai package). This PR is to ensure that @langhchain/openai package takes advantage of this new feature provided by openai.

It is possible that similar changes could happen in @langchain/azure-openai package (i.e. make sure it uses openai package instead of @azure/openai package). But, then there will be 2 packages (@langchain/openai and @langchain/azure-openai packages) and both depend/use the same openai package which will be redundant.

bracesproul · 2024-05-10T19:22:14Z

The @langchain/azure-openai package uses the @azure/openai SDK to access azure. Now, openai package itself has added features to support the same (independent of the @azure/openai package). This PR is to ensure that @langhchain/openai package takes advantage of this new feature provided by openai.

It is possible that similar changes could happen in @langchain/azure-openai package (i.e. make sure it uses openai package instead of @azure/openai package). But, then there will be 2 packages (@langchain/openai and @langchain/azure-openai packages) and both depend/use the same openai package which will be redundant.

But what are the features that would compel someone to use @langchain/openai to connect to Azure, instead of just using @langchain/azure-openai?

If it's just the ability to have one package installed, and use it to hit the OAI & Azure APIs, I don't think we want that since the packages are small enough already.

sarangan12 · 2024-05-10T19:41:59Z

@bracesproul Actually Yes. It is the reasoning that the users could just install one package (@langchain/openai) and get access to both OpenAI and Azure OpenAI and while doing this they could use the credentials (created in Azure) to access it. They do not have to use @langchain/azure-openai package at all.

Now, if you prefer I could make the same change in @langchain/azure-openai package. But, then you will have 2 packages - both using the openai package. What would be your recommendation to such a proposal?

achandmsft · 2024-05-10T22:30:34Z

@bracesproul,
We (Microsoft) worked closely with OpenAI (and Stainless) and I also touched upon this strategy briefly with @jacoblee93 from LangChainJS a few weeks ago. Reiterating here:

The goal here is to provide LangChainJS developers using OpenAI with the same quality of experience as LangChain (Python) developers. JavaScript developers want to seamlessly switch between OpenAI and Azure OpenAI endpoints using a single SDK without needing to change packages when talking to different OpenAI endpoints. All LangChain (Python) developers already enjoy this capability today without needing any Azure specific packages across the board: https://python.langchain.com/v0.1/docs/integrations/llms/azure_openai/#deployments.

In order to enable this experience, we first developed an Azure OpenAI client natively in the OpenAI SDK for JavaScript in order to be on par with Python. We just shipped this support a few days ago (https://www.npmjs.com/package/openai).

The problem with the additional azure specific package is not just the size, it's the fact that if there's a new feature in OpenAI that shows up in Azure OpenAI endpoint; they need to wait first for Azure OpenAI SDK for JavaScript to release a new version and then wait for the @langchain/azure-openai package to be updated to use this version. So they need to perform at least an additional package install/upgrade and if breaking, an additional code update. This is cumbersome (and error prone if there are versioning mismatches across these libraries as OpenAI APIs evolve rapidly).

Our solution provides a simple streamlined developer experience around OpenAI for LangChainJS customers. Hope this helps clarify.

bracesproul

Left a few comments. Good start, need to cleanup a couple nits. Thanks for working on this large PR!

Also, CI is failing, if you could resolve those errors that would be great!

libs/langchain-openai/src/azure/embeddings.ts

libs/langchain-openai/src/tests/azure/chat_models.int.test.ts

libs/langchain-openai/src/tests/azure/llms.int.test.ts

libs/langchain-openai/src/types.ts

Co-authored-by: Brace Sproul <[email protected]>

sarangan12 · 2024-05-13T22:31:12Z

@bracesproul I have addressed the feedback. Could you please take a look at the PR? Thanks

…update_openai_with_azure

jacoblee93 · 2024-05-13T23:29:35Z

I think consolidation is good - but what will become of the @azure/openai package then?

done

jacoblee93 · 2024-05-13T23:36:19Z

libs/langchain-openai/src/azure/llms.ts

@@ -27,15 +29,8 @@ export class AzureOpenAI extends OpenAI {
 configuration?: ClientOptions & LegacyOpenAIInput;
 }
 ) {
- // assume the base URL does not contain "openai" nor "deployments" prefix
- let basePath = fields?.openAIBasePath ?? "";


Actually, this is breaking, isn't it?

I don't think this is heavily used yet and there are no examples in our docs, so I'm ok with a change now.

jacoblee93 · 2024-05-14T00:01:28Z

Thanks! As noted above, it's technically breaking but this code path doesn't seem to be in our docs yet. Let's try to shim things in the future though once it becomes more prominent.

* Update OpenAI with Azure Specific Code * Export Embeddings * Add deployment parameters * Check to fix build issue * Update libs/langchain-openai/src/azure/embeddings.ts Co-authored-by: Brace Sproul <[email protected]> * Update libs/langchain-openai/src/azure/embeddings.ts Co-authored-by: Brace Sproul <[email protected]> * nit changes * Update Test Descriptions * Format * Fix types --------- Co-authored-by: Brace Sproul <[email protected]> Co-authored-by: jacoblee93 <[email protected]>

* Update OpenAI with Azure Specific Code * Export Embeddings * Add deployment parameters * Check to fix build issue * Update libs/langchain-openai/src/azure/embeddings.ts * Update libs/langchain-openai/src/azure/embeddings.ts * nit changes * Update Test Descriptions * Format * Fix types --------- Co-authored-by: Sarangan Rajamanickam <[email protected]> Co-authored-by: Brace Sproul <[email protected]>

Update OpenAI with Azure Specific Code

0fbf836

Merge remote-tracking branch 'upstream/main' into update_openai_with_…

720bfa8

…azure

sarangan12 marked this pull request as ready for review May 9, 2024 06:48

dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label May 9, 2024

sarangan12 changed the title ~~Update OpenAI with Azure Specific Code~~ [Not Ready for review yet] Update OpenAI with Azure Specific Code May 9, 2024

dosubot bot added the auto:improvement Medium size change to existing code to handle new use-cases label May 9, 2024

vercel bot deployed to Preview – langchainjs-docs May 9, 2024 06:48 View deployment

vercel bot deployed to Preview – langchainjs-api-refs May 9, 2024 06:49 View deployment

dosubot bot reviewed May 9, 2024

View reviewed changes

sarangan12 changed the title ~~[Not Ready for review yet] Update OpenAI with Azure Specific Code~~ Update OpenAI with Azure Specific Code May 9, 2024

Export Embeddings

dfd8620

vercel bot deployed to Preview – langchainjs-docs May 9, 2024 17:36 View deployment

vercel bot deployed to Preview – langchainjs-api-refs May 9, 2024 17:36 View deployment

Add deployment parameters

824894c

vercel bot deployed to Preview – langchainjs-api-refs May 9, 2024 18:23 View deployment

vercel bot deployed to Preview – langchainjs-docs May 9, 2024 18:23 View deployment

bracesproul reviewed May 9, 2024

View reviewed changes

bracesproul added the question Further information is requested label May 9, 2024

Merge branch 'main' into update_openai_with_azure

7644c6c

vercel bot deployed to Preview – langchainjs-api-refs May 12, 2024 21:47 View deployment

vercel bot deployed to Preview – langchainjs-docs May 12, 2024 21:47 View deployment

bracesproul previously requested changes May 12, 2024

View reviewed changes

sarangan12 and others added 3 commits May 13, 2024 19:34

Check to fix build issue

80c3a76

Update libs/langchain-openai/src/azure/embeddings.ts

53857f7

Co-authored-by: Brace Sproul <[email protected]>

Update libs/langchain-openai/src/azure/embeddings.ts

23f9f70

Co-authored-by: Brace Sproul <[email protected]>

vercel bot deployed to Preview – langchainjs-api-refs May 13, 2024 19:47 View deployment

vercel bot deployed to Preview – langchainjs-docs May 13, 2024 19:48 View deployment

nit changes

59d3bf4

vercel bot deployed to Preview – langchainjs-api-refs May 13, 2024 19:58 View deployment

vercel bot deployed to Preview – langchainjs-docs May 13, 2024 19:58 View deployment

Update Test Descriptions

42d608a

sarangan12 requested a review from bracesproul May 13, 2024 20:29

vercel bot deployed to Preview – langchainjs-docs May 13, 2024 20:29 View deployment

vercel bot deployed to Preview – langchainjs-api-refs May 13, 2024 20:29 View deployment

jacoblee93 changed the title ~~Update OpenAI with Azure Specific Code~~ openai[minor]: Update OpenAI with Azure Specific Code May 13, 2024

jacoblee93 added 2 commits May 13, 2024 16:26

Merge branch 'main' of https://github.com/hwchase17/langchainjs into …

9c40285

…update_openai_with_azure

Format

58da40e

jacoblee93 added lgtm PRs that are ready to be merged as-is and removed in progress PRs that are in progress but not ready to merge labels May 13, 2024

vercel bot deployed to Preview – langchainjs-api-refs May 13, 2024 23:34 View deployment

vercel bot deployed to Preview – langchainjs-docs May 13, 2024 23:34 View deployment

jacoblee93 reviewed May 13, 2024

View reviewed changes

Fix types

31d06ab

vercel bot deployed to Preview – langchainjs-api-refs May 13, 2024 23:45 View deployment

vercel bot deployed to Preview – langchainjs-docs May 13, 2024 23:45 View deployment

jacoblee93 merged commit aa65873 into langchain-ai:main May 14, 2024
19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

openai[minor]: Update OpenAI with Azure Specific Code #5323

openai[minor]: Update OpenAI with Azure Specific Code #5323

sarangan12 commented May 9, 2024 •

edited

vercel bot commented May 9, 2024 •

edited

dosubot bot May 9, 2024

dosubot bot May 9, 2024

dosubot bot May 9, 2024

dosubot bot May 9, 2024

bracesproul left a comment

sarangan12 commented May 10, 2024

bracesproul commented May 10, 2024

sarangan12 commented May 10, 2024

achandmsft commented May 10, 2024

bracesproul left a comment

sarangan12 commented May 13, 2024

jacoblee93 commented May 13, 2024

jacoblee93 May 13, 2024

jacoblee93 commented May 14, 2024

		@@ -48,6 +43,56 @@ export class AzureChatOpenAI extends ChatOpenAI {
		super(newFields);

		@@ -0,0 +1,829 @@
		import { test, jest, expect } from "@jest/globals";

		@@ -0,0 +1,333 @@
		import { test, expect } from "@jest/globals";

openai[minor]: Update OpenAI with Azure Specific Code #5323

openai[minor]: Update OpenAI with Azure Specific Code #5323

Conversation

sarangan12 commented May 9, 2024 • edited

vercel bot commented May 9, 2024 • edited

dosubot bot May 9, 2024

Choose a reason for hiding this comment

dosubot bot May 9, 2024

Choose a reason for hiding this comment

dosubot bot May 9, 2024

Choose a reason for hiding this comment

dosubot bot May 9, 2024

Choose a reason for hiding this comment

bracesproul left a comment

Choose a reason for hiding this comment

sarangan12 commented May 10, 2024

bracesproul commented May 10, 2024

sarangan12 commented May 10, 2024

achandmsft commented May 10, 2024

bracesproul left a comment

Choose a reason for hiding this comment

sarangan12 commented May 13, 2024

jacoblee93 commented May 13, 2024

jacoblee93 May 13, 2024

Choose a reason for hiding this comment

jacoblee93 commented May 14, 2024

sarangan12 commented May 9, 2024 •

edited

vercel bot commented May 9, 2024 •

edited