make ninja scons daemon scripts output to stderr as well as logfile. #4146
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mwichmann
reviewed
May 11, 2022
| ) | ||
|
|
||
| def log_error(msg): | ||
| logging.debug(msg) |
Collaborator
There was a problem hiding this comment.
- if it's an error, isn't
logging.errormore appropriate? - I know it's possible to configure logging to go both places, but if it needs to be log-level-dependent which goes where, I know it's not all that simple (multiple loggers, inheritance, etc. - may not be worth it). The simple case is to just add
stream=sys.stderrto the initialization inbasicConfig.
Contributor
Author
There was a problem hiding this comment.
-
The logging levels are useful when the volume of logging is overwhelming. Currently (IMHO) the ninja daemon doesn't put out enough logging for the logging levels to really be useful. Not saying logging.error is not more appropriate or correct, just kind of meaningless in this context and use case. We can easily change it in the future it the logging become hard to manage.
-
Maybe one day if the logging situation calls for it. For now I think this is fine.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed in appveyor run in #4144, intermittentally the ninja tests will fail. Unfortunately the ninja daemon prints most of its info to a logfile and not stderr, so when looking at the logs in the CI its hard to see what went wrong.
This PR puts the error message to stderr as well as the logs.
Contributor Checklist:
CHANGES.txt(and read theREADME.rst)