Pylons / pyramid

Pyramid - A Python web framework
https://trypyramid.com/
Other
3.97k stars 887 forks source link

pserve out function always prints to stderr #3710

Open markonose opened 2 years ago

markonose commented 2 years ago

In https://github.com/Pylons/pyramid/blob/master/src/pyramid/scripts/pserve.py#L146

The script outputs everything to the stderr file descriptor. If you redirect the stdout and stderr stream to your logger, then for example the 'Starting server' message gets logged as an error.

I would propose to split the out function into two in PServeCommand:

    def out(self, msg):  # pragma: no cover
        if self.args.verbose > 0:
            print(msg)

    def out_err(self, msg):  # pragma: no cover
        if self.args.verbose > 0:
            print(msg, file=sys.stderr)

And call the out_err function to print 'You must give a config file'.

In the rest of the file all of the other prints could also be changed to stdout, so that hey would no longer output starting/serving messages to stderr.

In other scripts even errors get printed to stdout, so this could also be applied in other scripts where applicable: https://github.com/Pylons/pyramid/blob/master/src/pyramid/scripts/prequest.py#L135

I can create a PR if such a changes would be permitted.

mmerickel commented 2 years ago

It's standard practice that logging systems log to stderr and leave stdout for core functionality. In the case of pserve everything is logging, leaving the core functionality to the wsgi server itself. I'm not sure this is really incorrect.

I would argue that prequest should be outputting several of those messages to stderr and could use some updates there.

markonose commented 2 years ago

Thank you for the quick reply.

Sorry my bad did not realize that it was standard practice to also output diagnostic data to stderr.

The idea behind my question was because we use pyramid with pserve, and in our app we redirect the stderr to our logger, in case something would go wrong it would get logged as an error. But after inspecting the code more closely, I can see that the only error 'You must give a config file' would get printed before the app even gets created, so it wouldn't get logged. So this was not really needed on our part.

Regarding prequest the error output should then be written to stderr and not stdout to keep things consistent. I would be happy to go through all of the commands and change the outputs to the correct stream if needed.

mmerickel commented 2 years ago

I think that'd be great if you wanted to do that!