-
Notifications
You must be signed in to change notification settings - Fork 482
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
Added Mongodb Atlas vector searcher #1624
Added Mongodb Atlas vector searcher #1624
Conversation
@@ -28,6 +29,7 @@ | |||
vector_searcher_implementations = { | |||
'lance': LanceVectorSearcher, | |||
'in_memory': InMemoryVectorSearcher, | |||
'mongodb': MongoAtlasVectorSearcher, |
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.
"mongodb_atlas": ...
superduperdb/vector_search/base.py
Outdated
|
||
class BaseVectorSearcher(ABC): | ||
def create_index(self, vi: 'VectorIndex', db: 'Datalayer'): |
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.
@abstractmethod
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.
Please explain the differences from this implementation and what was discussed here: #1605
superduperdb/vector_search/atlas.py
Outdated
print(json.dumps(index_definition, indent=2)) | ||
db.command(index_definition) | ||
|
||
def create_index(self, vi: 'VectorIndex', db: 'Datalayer'): |
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.
This function should be private and not called from outside.
In __init__
we need a check: "does this index exist". If not, then create.
See the description in the issue: #1605
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.
Looking good. Just a few things to address, then let's GTM!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1624 +/- ##
==========================================
- Coverage 80.33% 80.27% -0.07%
==========================================
Files 95 116 +21
Lines 6602 8218 +1616
==========================================
+ Hits 5304 6597 +1293
- Misses 1298 1621 +323 ☔ View full report in Codecov by Sentry. |
Description
fix #1605
Related Issues
Checklist
make unit-testing
andmake integration-testing
successfully?Additional Notes or Comments