-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
openai[patch]: Fix integration tests #5998
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -23,6 +25,9 @@ import { | |||
} from "@azure/identity"; |
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 noticed that a new line has been added to save the original value of the 'LANGCHAIN_CALLBACKS_BACKGROUND' environment variable. This is just a heads up for the maintainers to review this change. Let me know if you need further assistance with this.
@@ -1,3 +1,5 @@ | |||
/* eslint-disable no-process-env */ |
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 includes a comment disabling the eslint rule for direct usage of process.env
. This is flagged for your review to ensure proper handling of environment variables. Keep up the great work!
Tests which relied on callbacks are flaky if
LANGCHAIN_CALLBACKS_BACKGROUND
is set totrue
. This is due to those callback funcs not necessarily completing until after the parent fn (in this case a chat model) has resolved. To solve this, I wrapped those tests in atry { ... } finally { ... }
and setLANGCHAIN_CALLBACKS_BACKGROUND
to false at the start of the test, and inside thefinally
block I reset it to the original value.I also updated the Azure test
Test Azure ChatOpenAI with bearer token provider
to first check if the required env vars are set. If they are, it runs the test as normal. If not, it marks the test as skipped. Ideally we remove this code once we get our hands on those vars 🙃