-
-
Notifications
You must be signed in to change notification settings - Fork 335
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
Downgrade spurious 'error' logs #1119
Conversation
Thank you for seconding my case! I think either info or warning is fine here, but I think I would lean towards |
+1 to @krassowski , all changes can be made in |
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.
@ctcjab Thanks for opening a PR to fix this! Just one small comment below.
@krassowski @ctcjab Does there exist authoritative documentation on whether info or warning should be used in these logs? I don't have much experience with producing/consuming logs, so you two are the experts here.
Let's address this separately as this is a more complex topic, I opened #1120 to track this |
Since it is expected that not all model providers will be loadable, it is wrong to log the case that one cannot be loaded with level 'error'. Resolves jupyterlab#839.
53251f6
to
58c46a1
Compare
for more information, see https://pre-commit.ci
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.
@ctcjab Great work, thank you!
@meeseeksdev please backport to v3-dev |
Co-authored-by: ctcjab <[email protected]>
Since it is expected that not all embeddings model providers will be loadable, it is wrong to log the case that one cannot be loaded with level 'error'.
Resolves #839.