-
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
cloudflare[minor]: use native Cloudflare Workers AI binding #5287
cloudflare[minor]: use native Cloudflare Workers AI binding #5287
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
@@ -38,12 +38,11 @@ | |||
"author": "LangChain", |
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 this PR includes a change in dependencies, specifically the removal of "@cloudflare/ai" as a hard dependency and an update to the version of "@cloudflare/workers-types" as a dev dependency. I'm flagging this for your review to ensure it aligns with our dependency management strategy. Thanks!
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.
Thanks for taking this on, I have a couple comments to prevent breaking changes.
Hey so just got wind that the Rest API methods will be available in the main SDK: https://www.npmjs.com/package/cloudflare We should wait to land this until that is generally ready |
The general Updating https://github.com/Cherry/langchainjs/blob/4b9192176440b4acab9eac60a140200640d8bee2/libs/langchain-cloudflare/src/chat_models.ts#L78 to use the official SDK once the AI endpoints are available there makes sense, but https://github.com/Cherry/langchainjs/blob/fix/workers-ai-native-binding/libs/langchain-cloudflare/src/embeddings.ts seems to be intended for use only within a Worker. |
Yeah for sure but ideally we don't lose functionality for people using it outside workers - I've done that myself in demos. It should accept either a binding or a config object/client IMO. |
Agreed that would be ideal, but it doesn't look like it does today: https://github.com/langchain-ai/langchainjs/blob/main/libs/langchain-cloudflare/src/embeddings.ts. No functionality here will be lost, so perhaps that improvement would be better served in another PR? |
Hey, extremely sorry about the delay, this looks good! |
We are about to push minor versions of all packages related to the 0.3 release, will push this out alongside that. |
…n-ai#5287) Co-authored-by: Brace Sproul <braceasproul@gmail.com> Co-authored-by: jacoblee93 <jacoblee93@gmail.com>
As of 2024-04-11, there's no need to use the
@cloudflare/ai
package and it's deprecated:https://developers.cloudflare.com/workers-ai/changelog/
This swaps the code over to use the native binding directly, and updates
@cloudflare/workers-types
which now ships theAi
type. No other code changes are needed.