-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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-dynamic-sessions[major]: Add new package #5788
Conversation
* refactor: use azure identity refresh token * chore: formatting * refactor: lint issues * refactor: uniformize user agent prefix * feat: add default fallback to env variable * fix: incorrect template string * chore: add lc serialization * fix: token scope * docs: add .env.example * test: fix unit tests * test: update integration tests * refactor: rename integration tests
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -0,0 +1,43 @@ | |||
import type { ChatPromptTemplate } from "@langchain/core/prompts"; |
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 code changes in azure_dynamic_sessions-agent.ts
explicitly access an environment variable using process.env
. I've flagged this for your review to ensure it aligns with our environment variable usage guidelines. Keep up the great work! 🚀
@@ -0,0 +1,16 @@ | |||
import { SessionsPythonREPLTool } from "@langchain/azure-dynamic-sessions"; |
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 for the azure_dynamic_sessions.ts
file that explicitly accesses an environment variable via process.env
. Please review this change to ensure it aligns with our environment variable usage guidelines.
@@ -0,0 +1,92 @@ | |||
{ |
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 this PR includes changes to the dependencies, including additions and version constraints. This is flagged for your review to ensure the changes align with our dependency management strategy. Great work overall!
@@ -0,0 +1,135 @@ | |||
import { test, jest, describe, beforeEach } from "@jest/globals"; |
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 reviewed the code and noticed a new external HTTP request using fetch in the "Invoke with Python code" test. This comment is to flag the change for maintainers to review and ensure it aligns with the project's requirements.
@@ -0,0 +1,260 @@ | |||
import { promises as fs } from "node:fs"; |
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 wanted to flag that this PR introduces new external HTTP requests using the fetch
function. These requests are for making POST and GET requests to API endpoints, so it's important for maintainers to review this change. Let me know if you have any questions or need further clarification!
} | ||
|
||
const data = await fs.readFile( | ||
path.join(currentDir, "..", "package.json"), |
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.
I am not sure if this will work after build etc. in all environments - have been meaning to make this a formal API. Can we remove and readd later?
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.
Or just hardcode as a constant for now? Since it's just going to be 0.0.1
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.
Done
const userAgentPrefix = "langchainjs-azure-dynamic-sessions"; | ||
|
||
let userAgent = ""; | ||
async function getuserAgentSuffix(): Promise<string> { |
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.
nit: camel case
This is really neat! A few nits but seems good to me |
@jacoblee93 I've made the fixes, should be good now |
Thank you! |
This is live on npm as 0.1.0: https://www.npmjs.com/package/@langchain/azure-dynamic-sessions |
Add a new @langchain/azure-dynamic-sessions package that provides a Python code interpreter tool using code interpreter sandboxes in Azure Container Apps dynamic sessions.