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

Log to stderr instead of stdout by default #1709

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

frereit
Copy link

@frereit frereit commented Jan 14, 2025

This PR changes the default logging stream to sys.stderr when BLEAK_LOGGING is set.

There's nothing really wrong with logging to sys.stdout, it is a bit unusual in my opinion. It makes redirecting logging output to a file more difficult when there's already other output on stdout for example. Additionally, stdout is usually buffered, while stderr is usually unbuffered, which makes it a bit more convenient to work with for diagnostic information (e.g. grep & tail might work better).

I have opened this PR because I figured opening an issue first to ask for permission to open the PR would just take up more time. You may close this PR if this is not something you want to change 😄

@dlech
Copy link
Collaborator

dlech commented Jan 14, 2025

Sounds reasonable. Can you please also add a changelog entry?

@frereit
Copy link
Author

frereit commented Jan 14, 2025

Sounds reasonable. Can you please also add a changelog entry?

Sorry, done.

@frereit
Copy link
Author

frereit commented Jan 15, 2025

Hmm, I am unsure why the flake8 check fails: I did not touch any of that code, and it Works On My Machine ™️

$ git rev-parse HEAD
a8996c5830815f12b220a5b09058a17fcc648ec2
$ uv tool run flake8 . --count --show-source --statistics
0

@dlech
Copy link
Collaborator

dlech commented Jan 15, 2025

flake8 check fails

No worries. If you want to, you can add an extra commit to add E531 here. Or I will take care of it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants