Logging - Don't require psr/log for cv-lib. Fix STDERR/STDOUT routing.#169
Merged
totten merged 7 commits intocivicrm:masterfrom Jul 13, 2023
Merged
Logging - Don't require psr/log for cv-lib. Fix STDERR/STDOUT routing.#169totten merged 7 commits intocivicrm:masterfrom
cv-lib. Fix STDERR/STDOUT routing.#169totten merged 7 commits intocivicrm:masterfrom
Conversation
Before: Errors go to STDERR. Notices and debug msgs go to STDOUT.
After: All of them go to STDERR.
Comment:
* If you come from a logging POV, the old policy probably *sounds* right.
You're just taking a log-taxonomy of 8 buckets and mapping into the
log-taxonomy of 2 buckets. (Buckets are important - they help you decide
if something is important enough to look at!)
* If you come from a Unix scripting POV, this is insane. Sure, by default, they
both display to the console-user. The difference is comes when you chain
together commands. STDOUT goes to the next command, but STDERR stays with
the console-user.
* In Unix POV, the question should be: "What is the primary output-format of my program?"
* Ex: Suppose your command reads a database and outputs some JSON. Then...
do not, ever, for the love of god, send the general activity logs to
STDERR -- even if there are errors! It'll munge the JSON document!
* Ex: Suppose your command is a small network service, and you output a
request-log. Great - send that to STDOUT. It'll be easy for anyone
to record the log (`./my-program > my.log`). But then do not, for the
love of god, send any errorlogs to STDERR - they will not go the logfile!
* Ex: Suppose your command is interactive - you print some lines, then
ask for input, then print lines. Back+forth. But in this case,
there is no real difference STDERR and STDOUT -- they're displayed
as a single stream to the console-user.
Member
Author
|
civibot, test this please |
Member
Author
|
Most tests are passing. The failure appears to be random network I/O issues (e.g. |
Contributor
|
This works for me to resolve the drupal 10 problem in totten/civix#300. Thanks @totten. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This is a multi-prong update to logging. There are two major themes:
Don't require psr/log
In discussion about civix using cv-lib, we found that some environments have contention over the substance of
psr/log. (cv-libmay load its copy ofpsr/logbefore the UF starts, and UF's may have different expectations for the version ofpsr/log.) And the reality is thatpsr/loghas several major versions in the wild.So as a defensive move, it's better if
cv-libdoesn't have any specific expectations aboutpsr/log.Cleanup STDERR/STDOUT distinction
When you enable verbose output (
cv -vvv),cvhas been a bit confused about which information should go to STDOUT and which to STDERR.Since
cv(andcv-lib) are intended to be used in many kinds of scripts (including scripts with structured output - JSON, SQL, PHP, etc), it's bad default to send any logs to STDOUT. The verbose logs should go to STDERR.SO there are a few patches to make that change.