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 Mongodb full text search #1140

Closed
wants to merge 3 commits into from

Conversation

alperkaya
Copy link
Contributor

Related Issues

Proposed Changes:

It works in the same way as embedding retrieval works

How did you test it?

added new unit tests, and manual verification

Notes for the reviewer

Checklist

@alperkaya alperkaya requested a review from a team as a code owner October 16, 2024 13:42
@alperkaya alperkaya requested review from anakin87 and removed request for a team October 16, 2024 13:42
@github-actions github-actions bot added integration:mongodb-atlas type:documentation Improvements or additions to documentation labels Oct 16, 2024
@anakin87
Copy link
Member

anakin87 commented Oct 16, 2024

Hello and thanks for the contribution.
I will take a look in the next few days...

In the meantime:

  • let's make sure that all the tests pass in the CI
  • thoroughly test the feature locally
  • be aware that tests in test_embedding_retrieval (requiring a cloud MongoDB instance) are skipped when running from a fork, for security reasons
  • create a test_fulltext_retrieval file, similar to test_embedding_retrieval and containing integration tests (requiring a cloud MongoDB instance). They won't run in this PR but when everything else is fine, I will check them.

@alperkaya
Copy link
Contributor Author

Hello and thanks for the contribution. I will take a look in the next few days...

In the meantime:

* let's make sure that all the tests pass in the CI

* thoroughly test the feature locally

* be aware that tests in test_embedding_retrieval (requiring a cloud MongoDB instance) are skipped when running from a fork, for security reasons

* create a test_fulltext_retrieval file, similar to [test_embedding_retrieval](https://github.com/deepset-ai/haystack-core-integrations/blob/f95dd06614ed3089fa1fe4a0ce780b52949dc8e3/integrations/mongodb_atlas/tests/test_embedding_retrieval.py) and containing integration tests (requiring a cloud MongoDB instance). They won't run in this PR but when everything else is fine, I will check them.

Hello,

I have simplified the production code by removing the filter functionality to make the review process easier and more straightforward. Once this change is approved, I can reintroduce the filter functionality. Additionally, I tested the implementation using a connection to Atlas DB and included the sample data I used in the test code.

@kanenorman
Copy link
Contributor

Hi, @alperkaya. Are you still working on this PR?

If not, I would like to contribute.

@alperkaya
Copy link
Contributor Author

Hi, @alperkaya. Are you still working on this PR?

If not, I would like to contribute.

Hi @kanenorman, I am not working on this. Please contribute as you prefer ;)

@julian-risch
Copy link
Member

Closing this PR in favor of the more recent PR here: #1200

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration:mongodb-atlas type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Full-text MongoDBAtlas Retriever
4 participants