Skip to content
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

feat: support elasticsearch vector database #3558

Merged

Conversation

miendinh
Copy link
Contributor

@miendinh miendinh commented Apr 17, 2024

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Add Elasticsearch vector database for RAG, this feature requires package elasticsearch==7.15.2

Type of Change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update, included: Dify Document
  • Improvement, including but not limited to code refactoring, performance optimization, and UI/UX improvement
  • Dependency upgrade

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • TODO

Suggested Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
  • optional I have made corresponding changes to the documentation
  • optional I have added tests that prove my fix is effective or that my feature works
  • optional New and existing unit tests pass locally with my changes

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. 👻 feat:rag Embedding related issue, like qdrant, weaviate, milvus, vector database. 💪 enhancement New feature or request labels Apr 17, 2024
@crazywoola crazywoola marked this pull request as draft April 17, 2024 08:36
@takatost takatost requested a review from JohnJyong April 22, 2024 03:13
@miendinh miendinh marked this pull request as ready for review April 22, 2024 09:57
@miendinh miendinh changed the title feat: support elasticsearch vector database, working in process, not testing yet feat: support elasticsearch vector database Apr 22, 2024
@miendinh
Copy link
Contributor Author

hi @JohnJyong
do you have any comments?

@crazywoola
Copy link
Member

Hi there, please run ./dev/reformat in root dir. The CI build didn't pass.

@miendinh
Copy link
Contributor Author

Could you add `Setup ElasticSearch' to the CI like Milvus or Quant? @crazywoola

@JohnJyong
Copy link
Contributor

please fix the pytest and add vdb integration test for it , thanks @miendinh
intergration test folder : ‘tests.integration_tests.vdb’

@miendinh
Copy link
Contributor Author

please fix the pytest and add vdb integration test for it , thanks @miendinh intergration test folder : ‘tests.integration_tests.vdb’

added, but need ElasticSearch on CI to run ES tests, plz check @JohnJyong

@JohnJyong
Copy link
Contributor

CI workflow is set up in the .github/workflows/api-tests.yml @miendinh

@bowenliang123
Copy link
Contributor

Please test on your local development environment first.
Do not keep pushing untested code to run all the CI jobs, which is meaningless and costly.

@mboo2005
Copy link

@miendinh I tried your branch. Celery run failed because the config is missed.
error message:
image
Config is not interpreted
image

Copy link
Contributor

@bowenliang123 bowenliang123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall. But several changes are needed for response and actions.

docker/docker-compose.yaml Outdated Show resolved Hide resolved
web/yarn.lock Outdated Show resolved Hide resolved
docker/docker-compose.yaml Outdated Show resolved Hide resolved
@miendinh
Copy link
Contributor Author

miendinh commented Aug 1, 2024

plz check @JohnJyong @crazywoola, thanks.

@crazywoola
Copy link
Member

We will check it before v0.6.17

crazywoola
crazywoola previously approved these changes Aug 9, 2024
@crazywoola crazywoola merged commit f104b93 into langgenius:main Aug 13, 2024
6 checks passed
cuiks pushed a commit to cuiks/dify that referenced this pull request Sep 2, 2024
Co-authored-by: miendinh <[email protected]>
Co-authored-by: crazywoola <[email protected]>
Co-authored-by: crazywoola <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 enhancement New feature or request 👻 feat:rag Embedding related issue, like qdrant, weaviate, milvus, vector database. lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants