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

PGVector logger message level #4920

Merged
merged 1 commit into from
May 19, 2023
Merged

Conversation

jmtristancho
Copy link
Contributor

Change the logger message level

The library is logging at error level a situation that is not an error.
We noticed this error in our logs, but from our point of view it's an expected behavior and the log level should be warning.

@eyurtsev eyurtsev added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label May 18, 2023
@dev2049
Copy link
Contributor

dev2049 commented May 18, 2023

out of curiosity, wouldn't trying to delete a collection that's not there indicate a mistake in logic somewhere?

@jmtristancho
Copy link
Contributor Author

Could be, but it does not have to be always the case.
This class in particular has a init parameter pre_delete_collection: bool = False to delete the collection if it exists, so you can easily "clean" the embeddings of a collection (and the collection itself).
Using the param pre_delete_collection=True any operation would trigger the error in the log until the collection exists. It could be avoided checking if the collection exists, but that would imply an extra DB access and adding useless complexity to the code using this library.

@dev2049
Copy link
Contributor

dev2049 commented May 19, 2023

Could be, but it does not have to be always the case.
This class in particular has a init parameter pre_delete_collection: bool = False to delete the collection if it exists, so you can easily "clean" the embeddings of a collection (and the collection itself).
Using the param pre_delete_collection=True any operation would trigger the error in the log until the collection exists. It could be avoided checking if the collection exists, but that would imply an extra DB access and adding useless complexity to the code using this library.

thanks for explaining @jmtristancho! makes sense

@dev2049 dev2049 merged commit 729e935 into langchain-ai:master May 19, 2023
12 checks passed
@danielchalef danielchalef mentioned this pull request Jun 5, 2023
This was referenced Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm PR looks good. Use to confirm that a PR is ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants