chore(openai): update azure user agent #7543
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
dosubot
bot
added
the
size:XS
dosubot
bot
added
the
auto:nit
| "User-Agent": params.defaultHeaders?.["User-Agent"] | ||
| ? `${params.defaultHeaders["User-Agent"]}: langchainjs-azure-openai-v2` | ||
| : `langchainjs-azure-openai-v2`, | ||
| "User-Agent": `langchainjs-azure-openai/2.0.0 (node.js/${process.version}; ${process.platform}; ${process.arch}) ${params.defaultHeaders?.["User-Agent"] || ""}` |
There was a problem hiding this comment.
should the version be pulled from a variable, so this doesnt have to be reset on future releases? @sinedied ?
There was a problem hiding this comment.
Unfortunately we don't have a way to do this at the moment 😞
There was a problem hiding this comment.
@johnpapa the 2.0.0 is just to distinguish from the previous (deprecated) package implementation, it's not really a proper versioning as it won't change. It would be nice to send along the @langchain/openai package version though as Jacob mentioned it doesn't seem to be possible currently: from what I understand, we can't pull the version from package.json as the lib can also be ran from the browser where there's no FS access like with Node.js.
There was a problem hiding this comment.
Yeah we would need to manually some process to bump a statically specified version on release. We do it here:
https://github.com/langchain-ai/langsmith-sdk/blob/main/js/src/index.ts#L21
I just haven't had time to set it up
| "User-Agent": params.defaultHeaders?.["User-Agent"] | ||
| ? `${params.defaultHeaders["User-Agent"]}: langchainjs-azure-openai-v2` | ||
| : `langchainjs-azure-openai-v2`, | ||
| "User-Agent": `langchainjs-azure-openai/2.0.0 (node.js/${process.version}; ${process.platform}; ${process.arch}) ${params.defaultHeaders?.["User-Agent"] || ""}` |
There was a problem hiding this comment.
Yeah we would need to manually some process to bump a statically specified version on release. We do it here:
https://github.com/langchain-ai/langsmith-sdk/blob/main/js/src/index.ts#L21
I just haven't had time to set it up
| @@ -1,3 +1,4 @@ | |||
| import process from "node:process"; | |||
There was a problem hiding this comment.
Oh hmm actually this will break non-node environments - can we use this instead? https://github.com/langchain-ai/langchainjs/blob/main/langchain-core/src/utils/env.ts#L40
There was a problem hiding this comment.
My bad, I forgot about this!
I sent an update using getEnv that fixes that
dosubot
bot
added
size:S
| "User-Agent": params.defaultHeaders?.["User-Agent"] | ||
| ? `${params.defaultHeaders["User-Agent"]}: langchainjs-azure-openai-v2` | ||
| : `langchainjs-azure-openai-v2`, | ||
| "User-Agent": `langchainjs-azure-openai/2.0.0 (${env}) ${ |
There was a problem hiding this comment.
Actually, here you will have an extra space if no default user agent. How about:
const specifiedUserAgent = params.defaultHeaders?.["User-Agent"];
params.defaultHeaders = {
...params.defaultHeaders,
"User-Agent": `langchainjs-azure-openai/2.0.0 (${env})${specifiedUserAgent ? ` ${specifiedUserAgent}` : ""}`
};
There was a problem hiding this comment.
Thanks for the catch, I'll fix that!
|
Also looks like lint is failing: Error: 6:24 error '@langchain/core/utils/env' imported multiple times import/no-duplicates
Error: 7:40 error '@langchain/core/utils/env' imported multiple times import/no-duplicates |
|
@sinedied happy to merge as is, but LMK if you want to do this: #7543 (comment) |
|
I've fixed the extra space, should be good now! |
Changes
Fix library name and versioning, and add details about the Node.js version and platform in the user agent