Textualize / rich

Rich is a Python library for rich text and beautiful formatting in the terminal.
https://rich.readthedocs.io/en/latest/
MIT License
49.44k stars 1.72k forks source link

[BUG] Should RichHandler default to stderr #2022

Closed rdburns closed 2 years ago

rdburns commented 2 years ago

Describe the bug

"Bug" is a little strong, but I noticed that by default Python logging writes to stderr, but when using RichHandler by default it writes to stdout.

I know you can get around this with something like:

logging.basicConfig(
    level="DEBUG", datefmt="[%X]", handlers=[RichHandler(console=Console(stderr=True))]
)

But would it be more sensible and less surprising for new users if by default RichHandler logged to stderr?

willmcgugan commented 2 years ago

If you are implying it is not sensible and is surprising, I'd like to know why...

willmcgugan commented 2 years ago

Will need an argument one way or the other...

rdburns commented 2 years ago

I just found that it seemed unexpected if python logging writes to stderr by default, I expected the Rich Handler to do the same. Isn’t that generally what one would expect? I might be wrong about that. I don’t think it’s a huge issue, but in my app I was piping the output somewhere and it’s nice for logging to not be mixed into the output stream if it’s a unix-y style terminal application.

On Wed, Mar 9, 2022, at 8:13 AM, Will McGugan wrote:

Will need an argument one way or the other...

— Reply to this email directly, view it on GitHub https://github.com/Textualize/rich/issues/2022#issuecomment-1062908290, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACRMYWCSEEZCIWFQZ34VELU7CPYZANCNFSM5P3AQEDQ. You are receiving this because you authored the thread.Message ID: @.***>

willmcgugan commented 2 years ago

It would be surprising for some users if they piped the output and it didn't appear. Having the handler share an internal console also allows for logging to be used in conjunction with progress bars. Like you said, its trivial to make it work however you want.

github-actions[bot] commented 2 years ago

Did I solve your problem?

Why not buy the devs a coffee to say thanks?

imdoroshenko commented 2 years ago

This bug caused issues in my CLI app because logs were mixed with the app output and caused errors when piping. I feel that by default rich should avoid unexpected behaviour.

robinbowes commented 1 year ago

I also think that the RichHandler should not change default logger behaviour, ie. it should log to stderr by default.

It would be surprising for some users if they piped the output and it didn't appear

Anyone using RichHandler is already using python logging, and therefore expecting log output to go to stderr. It is more surprising for logging to not go to stderr.

This is a bug, IMO.

frans-fuerst commented 11 months ago

I'd also consider this a bug - lots (if not most) of CLI tools write errors and debug to stderr while the (often parse-able) output goes to stdout and might be piped to another tool, which now would have to guess whether a line is (optional) logging output, some error message or actual data it should process. E.g.:

ssh -vv <user>@<machine> <command> | <next-tool> | <next-tool> ..

..would be quite hard to handle if every tool needed to know the (often flexible) logging format of it's predecessor.

(This is true for every other (at least Posix based) tool I know, correct me if I'm wrong)

If you need both stderr and stdout you can easily redirect them using e.g. 2>&1. But without logging/error messages being sent to stderr there is no trivial way to separate them.

ryandward commented 8 months ago

Lovely package, but I was getting invalid json output for the reasons above. Pretty unexpected imo.

simonwiles commented 7 months ago

Yeah, this just cost me longer than I'd care to admit before I realized that RichHandler in the mix was the cause of my problem. For my part I can't see a single good argument for changing the behaviour of the logging module in this respect.

rdburns commented 7 months ago

For the record i am still holding my initial opinion that logging should default to stderr as it does normally with the logging module.

On Fri, Mar 15, 2024, at 4:04 PM, Simon Wiles wrote:

Yeah, this just cost me longer than I'd care to admit before I realized that RichHandler in the mix was the cause of my problem. For my part I can't see a single good argument for changing the behaviour of the logging module in this respect.

— Reply to this email directly, view it on GitHub https://github.com/Textualize/rich/issues/2022#issuecomment-2000370055, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACRMYUTY54K2S4MTK2RG6DYYNH3NAVCNFSM5P3AQED2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBQGAZTOMBQGU2Q. You are receiving this because you authored the thread.Message ID: @.***>