-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Python: Introducing AzureCosmosDBforMongoDB store and collection #10609
Conversation
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.
Copilot reviewed 6 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (4)
- python/tests/unit/connectors/memory/mongodb_atlas/test_mongodb_atlas_collection.py: Evaluated as low risk
- python/semantic_kernel/connectors/memory/azure_cosmos_db/const.py: Evaluated as low risk
- python/samples/concepts/memory/new_memory.py: Evaluated as low risk
- python/semantic_kernel/connectors/memory/azure_cosmos_db/init.py: Evaluated as low risk
Comments suppressed due to low confidence (3)
python/semantic_kernel/connectors/memory/azure_cosmos_db/azure_cosmos_db_mongodb_store.py:8
- The override decorator is imported but not used in the file. Remove the unused import statement.
from typing import override # pragma: no cover
python/semantic_kernel/connectors/memory/mongodb_atlas/mongodb_atlas_collection.py:169
- [nitpick] The log message could be more descriptive. Consider changing it to 'Upserted records: %s' % result.
logger.debug("Upserted records:", result)
python/semantic_kernel/connectors/memory/mongodb_atlas/mongodb_atlas_collection.py:170
- Add a check for 'result.upserted_ids' before iterating over it to avoid potential errors.
return [str(value) for key, value in result.upserted_ids.items()]
python/semantic_kernel/connectors/memory/azure_cosmos_db/azure_cosmos_db_mongodb_settings.py
Outdated
Show resolved
Hide resolved
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.
Copilot reviewed 6 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (4)
- python/tests/unit/connectors/memory/mongodb_atlas/test_mongodb_atlas_collection.py: Evaluated as low risk
- python/samples/concepts/memory/new_memory.py: Evaluated as low risk
- python/semantic_kernel/connectors/memory/azure_cosmos_db/const.py: Evaluated as low risk
- python/semantic_kernel/connectors/memory/azure_cosmos_db/init.py: Evaluated as low risk
Comments suppressed due to low confidence (1)
python/semantic_kernel/connectors/memory/mongodb_atlas/mongodb_atlas_store.py:72
- [nitpick] The logic for setting 'managed_client' in the constructor is inconsistent with the logic used in 'AzureCosmosDBforMongoDBCollection'. Consider updating for consistency.
managed_client = kwargs.get("managed_client", not mongo_client)
python/semantic_kernel/connectors/memory/mongodb_atlas/mongodb_atlas_collection.py
Outdated
Show resolved
Hide resolved
6c05940
to
d07c6bc
Compare
python/semantic_kernel/connectors/memory/azure_cosmos_db/azure_cosmos_db_mongodb_collection.py
Outdated
Show resolved
Hide resolved
python/semantic_kernel/connectors/memory/azure_cosmos_db/azure_cosmos_db_mongodb_collection.py
Outdated
Show resolved
Hide resolved
python/semantic_kernel/connectors/memory/azure_cosmos_db/azure_cosmos_db_mongodb_collection.py
Show resolved
Hide resolved
python/semantic_kernel/connectors/memory/azure_cosmos_db/azure_cosmos_db_mongodb_settings.py
Outdated
Show resolved
Hide resolved
python/semantic_kernel/connectors/memory/azure_cosmos_db/azure_cosmos_db_mongodb_store.py
Show resolved
Hide resolved
python/semantic_kernel/connectors/memory/azure_cosmos_db/azure_cosmos_db_mongodb_collection.py
Outdated
Show resolved
Hide resolved
8b39b6f
to
21402e0
Compare
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.
Will there be unit and integration tests for the new connector in a separate PR?
21402e0
to
b77b19b
Compare
Motivation and Context
Adds Azure Cosmos DB for MongoDB support using the new vector store model.
Builds on the MongoDB Atlas integration, replaced the index creation and settings to make it work.
Closes #6836
Description
Contribution Checklist