-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
azure-openai[minor]: update docs and fix openai api usage #4898
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -266,15 +266,27 @@ export class AzureChatOpenAI | |||
(getEnvironmentVariable("AZURE_OPENAI_API_KEY") || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there! I noticed that the recent change in the code explicitly accesses an environment variable using getEnvironmentVariable("AZURE_OPENAI_API_KEY")
. I've flagged this for your review to ensure it aligns with the intended functionality. Keep up the great work!
@@ -64,16 +63,28 @@ export class AzureOpenAIEmbeddings |
There was a problem hiding this comment.
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 a change in the PR for maintainers to review. The added code explicitly accesses an environment variable using the getEnvironmentVariable
function, so it's important to ensure that this change is handled correctly. Keep up the great work! 🚀
@@ -133,15 +133,27 @@ export class AzureOpenAI< | |||
(getEnvironmentVariable("AZURE_OPENAI_API_KEY") || |
There was a problem hiding this comment.
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 a change in the PR that explicitly requires an environment variable "AZURE_OPENAI_API_KEY" using the getEnvironmentVariable
function. This is to ensure the maintainers review and validate the handling of environment variables in the code.
@@ -16,6 +16,8 @@ import { | |||
import { CallbackManager } from "@langchain/core/callbacks/manager"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there! I noticed that the recent change in the code added an import for getEnvironmentVariable
from "@langchain/core/utils/env", which suggests the code is accessing environment variables. I've flagged this for your review. Keep up the great work!
@@ -68,3 +69,16 @@ test("Test OpenAIEmbeddings.embedQuery with TokenCredentials", async () => { | |||
const res = await embeddings.embedQuery("Hello world"); |
There was a problem hiding this comment.
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 added test explicitly accesses an environment variable using getEnvironmentVariable
. I've flagged this for your review to ensure it aligns with the project's requirements. Let me know if you need further assistance!
@@ -325,3 +326,18 @@ test("Test OpenAI with Token credentials ", async () => { | |||
const res = await model.invoke("Print hello world"); |
There was a problem hiding this comment.
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 the recent change in the PR for review. It looks like the code is explicitly accessing and using an environment variable, so it's important to ensure everything is handled correctly. Keep up the great work!
docs: add Azure OpenAI SDK documentation docs: update docs and example for new package name docs: update readme docs: add Azure LLM integrations docs: update doc docs: add managed identity usage fix: add missing model name in example docs: add Azure OpenAI embeddings docs
BREAKING CHANGE: stripNewLines option is now disabled by default
I'm not sure why the docs deployment are failing, is there a command I can run locally to see what's wrong? |
Thank you! Build failure looks like a flake. |
Going through the docs I noticed the Azure OpenAI docs were not up to date with the latest version I had locally, a rebase may have gone wrong with the initial PR that created the package. So here's the updated docs.
I also noticed using an OpenAI API key was not working, so I fixed that too and added some tests for it.