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

Write all log output to stderr (do not pollute stdout) #1337

Merged
merged 2 commits into from
Oct 21, 2022
Merged

Write all log output to stderr (do not pollute stdout) #1337

merged 2 commits into from
Oct 21, 2022

Conversation

aryoda
Copy link
Contributor

@aryoda aryoda commented Oct 18, 2022

  • Affects logger.info and logger.debug
  • complies now to the behavior of pythons's standard logging package

Background

Our logging system pollutes the normal (non-logging) stdout output with logging output, we need a clear separation here
(eg. logging to stderr instead of stdout) otherwise we can never separate normal from (partially random) logging output in unit tests.

Standard python's logger package says:

If you call the functions debug(), info(), warning(), error() and critical(), they will check to see if no destination is set; and if one is not set, they will set a destination of the console (sys.stderr)

https://docs.python.org/3/howto/logging.html

* Affects logger.info and logger.debug
* complies now to the behavior of pythons's standard logging package
for line in tools.wrapLine(msg):
syslog.syslog(syslog.LOG_INFO, 'INFO: ' + line)

def debug(msg, parent = None, traceDepth = 0):
if DEBUG:
msg = '%s %s' %(_debugHeader(parent, traceDepth), msg)
print('%sDEBUG%s: %s' %(bcolors.OKBLUE, bcolors.ENDC, msg), file = sys.stdout)
# Why does this code differ from eg. "error()"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In error() an error message is written to syslog no matter if debug mode is on or not. But if DEBUG then debug infos are added to the error messgae.

In debug() there is no need to add debug infos to the message because the message itself is of type DEBUG.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is irritating to me is that DEBUG is not a severity threshold for logging but a flag to inject additional call stack information ("_debugHeader") but for a single severity level debug() it is also a threshold.
I do not know any logging framework that works like that (and don't understand why this makes sense).
Perhaps the idea was to work like -v vs. -vv verbosity switches (the more "v"s the chattier is the output ;-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we misunderstand here each other? e.g. in Python's own logging package logging.DEBUG is a threshold.
But you are deeper in that part of the code. It was just a quick and dirty night shift thought. 😄

@@ -55,14 +55,16 @@ def warning(msg , parent = None, traceDepth = 0):
def info(msg , parent = None, traceDepth = 0):
if DEBUG:
msg = '%s %s' %(_debugHeader(parent, traceDepth), msg)
print('%sINFO%s: %s' %(bcolors.OKGREEN, bcolors.ENDC, msg), file=sys.stdout)
print('%sINFO%s: %s' %(bcolors.OKGREEN, bcolors.ENDC, msg), file=sys.stderr)
for line in tools.wrapLine(msg):
syslog.syslog(syslog.LOG_INFO, 'INFO: ' + line)

def debug(msg, parent = None, traceDepth = 0):
if DEBUG:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would revert that if.

def debug()
    if not DEBUG:
        return

In that case you can save the indention of the following lines.
Clearer code IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Yes, would be possible, but why change existing code that works ;-)

@emtiu emtiu merged commit 4848708 into bit-team:master Oct 21, 2022
aryoda added a commit that referenced this pull request Nov 5, 2022
… to #1333)

* Bug was introduced with #1337 (write all log output to stderr)
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.

3 participants