-
Notifications
You must be signed in to change notification settings - Fork 0
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
Nuke Chroma #49
Nuke Chroma #49
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.
The pull request effectively removes dependencies on the ChromaDB and introduces a new PGDB class for handling Postgres databases. However, there are several TODOs and missing error handling that need to be addressed to ensure robustness and functionality. Completing these items will be crucial for maintaining data integrity and handling database interactions effectively.
Butler is in closed beta. Reply with feedback or to ask Butler to review other parts of the PR. Please give feedback with emoji reacts.
ragdaemon/database/__init__.py
Outdated
# # In case the api key is wrong, try to embed something to trigger an error. | ||
# _ = db.add(ids="test", documents="test doc") | ||
# db.delete(ids="test") | ||
db = PGDB(cwd=cwd, db_path=db_path) |
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.
It seems like the PGDB
class is being used here without handling exceptions specifically for Postgres-related errors. Consider adding specific error handling for Postgres exceptions to provide more detailed feedback about database connection issues or query failures.
ragdaemon/database/pg_database.py
Outdated
for k, v in metadata.items(): | ||
if k in self.fields: | ||
remote_records[id][k] = v | ||
# TODO: Update remote records |
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.
The TODO
comment here suggests that the implementation for updating remote records is not yet complete. It's important to address this to ensure data consistency between local and remote databases.
ragdaemon/database/pg_database.py
Outdated
metadatas: list[dict] | dict, | ||
documents: list[str] | str, | ||
) -> list[str]: | ||
# TODO: Fetch remote records for ids |
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.
There is another TODO
comment indicating that fetching remote records for IDs is not implemented. This needs to be completed to ensure that the add
method functions correctly when integrating with a remote Postgres database.
No description provided.