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.

Sign up for GitHub

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

openai[patch],azure-openai[patch]: Update legacy classes to respect the apiKey field #5714

Merged
merged 4 commits into from
Jun 11, 2024
Merged

openai[patch],azure-openai[patch]: Update legacy classes to respect the apiKey field #5714

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dcec704
Fixed to match new apiKey field
scottforte Jun 10, 2024
63063d5
Merge branch 'main' of https://github.com/hwchase17/langchainjs into …
jacoblee93 Jun 11, 2024
3f89ad5
Format and deprecate
jacoblee93 Jun 11, 2024
2f3d9de
Fix type
jacoblee93 Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion langchain/src/chains/openai_moderation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ export class OpenAIModerationChain
super(fields);
Copy link

Choose a reason for hiding this comment

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

Hey there! I've flagged this PR for your review as 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 handling practices.

this.throwError = fields?.throwError ?? false;
this.openAIApiKey =
fields?.openAIApiKey ?? getEnvironmentVariable("OPENAI_API_KEY");
fields?.apiKey ??
fields?.openAIApiKey ??
getEnvironmentVariable("OPENAI_API_KEY");

if (!this.openAIApiKey) {
throw new Error("OpenAI API key not found");
Expand Down
4 changes: 3 additions & 1 deletion libs/langchain-azure-openai/src/chat_models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,9 @@ export class AzureChatOpenAI
getEnvironmentVariable("AZURE_OPENAI_API_DEPLOYMENT_NAME");
Copy link

Choose a reason for hiding this comment

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

Hey there! 👋 I've reviewed the code and noticed that the recent change explicitly accesses environment variables using getEnvironmentVariable. I've flagged this for your review to ensure it aligns with our best practices. Let me know if you have any questions!


const openAiApiKey =
fields?.openAIApiKey ?? getEnvironmentVariable("OPENAI_API_KEY");
fields?.apiKey ??
fields?.openAIApiKey ??
getEnvironmentVariable("OPENAI_API_KEY");

this.azureOpenAIApiKey =
fields?.apiKey ??
Expand Down
4 changes: 3 additions & 1 deletion libs/langchain-azure-openai/src/embeddings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ export class AzureOpenAIEmbeddings
getEnvironmentVariable("AZURE_OPENAI_API_ENDPOINT");
Copy link

Choose a reason for hiding this comment

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

Hey there! I've reviewed the code and flagged a change in the diff for you to review. It looks like the code is explicitly accessing and requiring an environment variable, so it's important to double-check this change. Let me know if you need any further assistance!


const openAiApiKey =
fields?.openAIApiKey ?? getEnvironmentVariable("OPENAI_API_KEY");
fields?.apiKey ??
fields?.openAIApiKey ??
getEnvironmentVariable("OPENAI_API_KEY");

this.azureOpenAIApiKey =
fields?.apiKey ??
Expand Down
4 changes: 3 additions & 1 deletion libs/langchain-azure-openai/src/llms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ export class AzureOpenAI<
getEnvironmentVariable("AZURE_OPENAI_API_DEPLOYMENT_NAME");
Copy link

Choose a reason for hiding this comment

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

Hey team, just a heads up that I've flagged a change in the PR for review. The added line getEnvironmentVariable("OPENAI_API_KEY"); explicitly accesses an environment variable, so it's worth double-checking to ensure everything is handled correctly.


const openAiApiKey =
fields?.openAIApiKey ?? getEnvironmentVariable("OPENAI_API_KEY");
fields?.apiKey ??
fields?.openAIApiKey ??
getEnvironmentVariable("OPENAI_API_KEY");

this.azureOpenAIApiKey =
fields?.apiKey ??
Expand Down
4 changes: 3 additions & 1 deletion libs/langchain-openai/src/legacy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ export class OpenAIChat
super(fields ?? {});
Copy link

Choose a reason for hiding this comment

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

Hey team, I've flagged this change for review as it appears to add, access, and read environment variables using getEnvironmentVariable and process.env. It's important to ensure that environment variable handling is secure and follows best practices.


this.openAIApiKey =
fields?.openAIApiKey ?? getEnvironmentVariable("OPENAI_API_KEY");
fields?.apiKey ??
fields?.openAIApiKey ??
getEnvironmentVariable("OPENAI_API_KEY");

this.azureOpenAIApiKey =
fields?.azureOpenAIApiKey ??
Expand Down