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

pelican --quiet produces non critical output #3201

Open
arterrey opened this issue Oct 5, 2023 · 7 comments
Open

pelican --quiet produces non critical output #3201

arterrey opened this issue Oct 5, 2023 · 7 comments
Labels

Comments

@arterrey
Copy link

arterrey commented Oct 5, 2023

In the --help it says

-q, --quiet           Show only critical errors. (default: None)

However, when I run pelican with --quiet I get non critical output.

$ pelican --quiet
Done: Processed 1 article, 0 drafts, 0 hidden articles, 2 pages, 1 hidden page and 0 draft pages in 0.15 seconds.

Here is my version...

$ pelican --version
4.8.0
@arterrey arterrey added the bug label Oct 5, 2023
@justinmayer
Copy link
Member

justinmayer commented Oct 5, 2023 via email

@copperchin
Copy link
Contributor

copperchin commented Oct 5, 2023

The --quiet flag, along with --debug and --verbose, only affect logging output.

--quiet does silence all but critical logging output; however: any console output that does not use logging (eg. console.print()) is entirely unaffected by the verbosity flags.

As far as I can tell, the only place where the console is directly written to is in pelican/__init.py, typically at the start and/or end of a run pelican command. This is to show what command is being run and what the result was.


I'm not clear on what's being asked for here, or the use case.

  • An updated help message ("Suppress output from console, showing only summaries. (default: None)"), to be more explicit?
  • A need to suppress all output, including confirmation of a successful run, because...?
  • Something else?

@arterrey
Copy link
Author

arterrey commented Oct 7, 2023

"console.print()) is entirely unaffected by the verbosity flags." I would agree with that.

However, a summary of the activity of the tool is really INFO level log output not console output. If the summary text were the actual output of the tool then I would agree that console.print() is correct, but the output is the building of a site not the summary text, so IMO it should be sent to the logger.

In my case I'm layering automations on top of this and I only really care to see output if it is an error. So I'm tempted to write pelican --quiet > /dev/null except that because there is output, I can't know for certain that there are not additional error messages going to SDTOUT that I need to know about.

If it isn't practical to move the summary message to INFO logging, then i would suggest squashing the console.print in just the pelican/__init.py case when the user uses --quiet

Updating the help message as you suggest makes the feature not useful, additionally it isn't correct as you point out, if a plugin or the pelicancon.py were to make calls to console.print then it wouldn't be squashed.

@copperchin
Copy link
Contributor

copperchin commented Oct 10, 2023

So I (incorrectly) assumed that the logging output was going to stderr, but that doesn't seem to be the case.

I'd like to change the default logging StreamHandler to output to stderr, leaving the console object to continue outputting to stdout. This would allow developers to easily separate user-friendly (and logging-independent) messages from log warnings/errors.

Automation could then check the stderr stream (ignoring the stdout). Do you see that being a valid solution to your situation?

Alternatively something you can try under the current implementation is running pelican with --fatal errors.
This will halt processing and quit with a non-zero exit code on error-level (and above) log messages,

@avaris
Copy link
Member

avaris commented Oct 31, 2023

I'd like to change the default logging StreamHandler to output to stderr, leaving the console object to continue outputting to stdout. This would allow developers to easily separate user-friendly (and logging-independent) messages from log warnings/errors.

I also initially assumed the logging would go to stderr but rich doesn't do that. I suspect, by default, having everything handled with a single console is important for interactive functionality (progress bars etc). So, splitting them could potentially bring a set of other issues along.

Maybe the cleanest solution is switching from console.prints to using logging for everything. But, I think we would need a custom level for that. We are already using INFO for other things and default verbosity is WARNING (or >INFO effectively). And, I'd like to have these still displayed by default. So we need something in between INFO and WARNING.

@copperchin
Copy link
Contributor

I also initially assumed the logging would go to stderr but rich doesn't do that. I suspect, by default, having everything handled with a single console is important for interactive functionality (progress bars etc). So, splitting them could potentially bring a set of other issues along.

There's actually already two separate console objects, so there's no need to split anything further than it already is. The only change needed would be to specify the logger's console handler to output to stderr rather than the default (stdout).

Maybe the cleanest solution is switching from console.prints to using logging for everything.

What's desirable here is the ability to differentiate between logging output and user-friendly terminal (console) outputs.
Switching all output to logging actually makes the problem worse -- we'd have no way of distinguishing the two if we stopped using pelican.log.console object.

This is a pretty trivial change to implement: we don't need to introduce new custom log levels; it'd literally be a one line change in log initialization.

I'm happy to implement this change myself, but as I'm already making some big changes to logging setup (#3038), I'd like to get those changes merged first so I don't keep having to deal with merge conflicts ':)

@avaris
Copy link
Member

avaris commented Nov 1, 2023

There's actually already two separate console objects,

Where? Currently, there is only single Console() instance that is instantiated in pelican.log. Everything uses that one.

so there's no need to split anything further than it already is. The only change needed would be to specify the logger's console handler to output to stderr rather than the default (stdout).

If you split them, I'd suggest testing to see how this behaves. I suspect it will not like it...

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

No branches or pull requests

4 participants