search

Nukesor / pueue

:stars: Manage your shell commands.
Apache License 2.0
5.03k stars 133 forks source link

[Bug] daemon version warning is written to stdout, not stderr #562

Closed mjpieters closed 2 months ago

mjpieters commented 3 months ago

Describe the bug

When client and daemon versions differ, all output except for JSON output is prepended with

Different daemon version detected '{version}'. Consider restarting the daemon.

This message is written to stdout, which means it can't easily be filtered out.

Steps to reproduce

Run the daemon as version 3.4.0, then use client version 3.4.1 and use --print-task-id to only get the task ID to be output:

$ pueue add --print-task-id 'echo hello' 2>/dev/null
Different daemon version detected '3.4.0'. Consider restarting the daemon.
1

The expected output would be 1, with Different daemon version detected '3.4.0'. Consider restarting the daemon. having been redirected to /dev/null.

Debug logs (if relevant)

No response

Operating system

Debian 10

Pueue version

3.4.0 (daemon) and 3.4.1 (client)

Additional context

At the very least, the message should be written to stderr. Ideally, it should be possible to disable it altogether (pueue --no-warnings add ...).

Nukesor commented 3 months ago

Thinking about it, all logs should probably be sent to stderr and the same should probably be true for pueued as well?

mjpieters commented 3 months ago

Thinking about it, all logs should probably be sent to stderr and the same should probably be true for pueued as well?

Hard to say. Generally speaking, 'out of band' information should go to stderr. When I run pueue log <TASKID> the goal is to get the log output of a task, so that should go to stdout. Anything else should go to stderr.

Nukesor commented 3 months ago

Ah yep. By logs I meant all of the stuff that's printed when pueue/d is called with -v/v/v

pueue log should definitely be put to stdout

Nukesor commented 3 months ago

The linked MR sends all log output to stderr and sends an error log message in case of version incompatibilities.

I'm not sure about the error-level "version incompatiblity" message though. It feels more like a warning, but the default log level is error. Maybe the default log level for the client should be bumped to warning?

mjpieters commented 3 months ago

Yes, that's a warning level message. What other warning-level log output could the client produce? I'd review that and bump the level to warning if there isn't too much else that'd show up.