-
-
Notifications
You must be signed in to change notification settings - Fork 486
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
Set Bokeh logging to INFO #4916
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4916 +/- ##
==========================================
- Coverage 81.73% 81.25% -0.49%
==========================================
Files 326 326
Lines 48006 48003 -3
==========================================
- Hits 39236 39003 -233
- Misses 8770 9000 +230 ☔ View full report in Codecov by Sentry. |
I want to carefully review this as I have code that relies on logging and have found that setting this up is difficult in general. |
@maximlt How's the careful review going? 😄 |
It looks like I have carefully managed not to get to review it :( I promise this will be done this week! |
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.
(I rebased this PR to be able to test it).
I'm -1 on the change as is. It's setting the log level of Bokeh's logger to INFO
, ignoring what the user may have configured via BOKEH_PY_LOG_LEVEL
or --log-level
or whatever logger customisation they might have done in a --setup
file.
I opened an issue to report the problem upstream dask/distributed#8750.
There's in fact already a PR to stop setting the logs on import dask/distributed#8634. |
Fixes #2302
Beforehand, this will not output logs because
distributed
sets the logger level to error. Not sure if we need to have a setting for this.