-
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
community[minor]: JinaAI embeddings integration #5842
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -0,0 +1,12 @@ | |||
import { JinaEmbeddings } from "@langchain/community/embeddings/jina"; |
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 noticed that the recent code addition in jina.ts
explicitly accesses an environment variable using process.env
. I've flagged this for your review to ensure it aligns with our best practices for handling environment variables. Keep up the great work!
@@ -0,0 +1,11 @@ | |||
import { JinaEmbeddings } from "@langchain/community/embeddings/jina"; |
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! This PR adds a new HTTP request using the embedQuery
method to retrieve embeddings from the Jina API. I've flagged this for your review to ensure it aligns with our API usage guidelines. Let me know if you have any questions or need further clarification!
@@ -0,0 +1,11 @@ | |||
import { JinaEmbeddings } from "@langchain/community/embeddings/jina"; |
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 the recent change in the PR for maintainers to review, as it explicitly accesses an environment variable using process.env
. This is an important security consideration, so it's good to double-check. Let me know if you need further assistance!
@@ -0,0 +1,125 @@ | |||
import { existsSync, readFileSync } from "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 there! I noticed that this PR introduces a new HTTP request using the fetch
function to make a POST request to the JINA_API_URL
endpoint. I've flagged this change for your review to ensure it aligns with the project's requirements. Let me know if you have any questions or need further clarification!
@@ -0,0 +1,125 @@ | |||
import { existsSync, readFileSync } from "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.
Great work on the PR! I've flagged this change for review by the maintainers as it involves accessing environment variables using the getEnvironmentVariable
function. Keep up the good work!
Thank you! |
Description
Implemented a new embedding integration that makes JinaAI's embedding models available on langchainjs. A new class named JinaEmbeddings is created, which wraps the inference API.
This PR includes an example and integration tests related to the newly implemented class. Apart from the new integration's configuration, the original open-source components are intentionally left unchanged to avoid introducing any breaking changes.