When a command fails, print its error in multiple lines #178
Conversation
This file contains 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
querti
force-pushed
the
improve-command-logs
branch
from
f5706ea
to
2ef2366
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #178 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 28 28
Lines 3026 3078 +52
=========================================
+ Hits 3026 3078 +52
☔ View full report in Codecov by Sentry. |
querti
force-pushed
the
improve-command-logs
branch
from
2ef2366
to
8fd3257
Compare
zxiong
approved these changes
Jul 26, 2023
emilyzheng
reviewed
Aug 1, 2023
| @@ -182,7 +182,9 @@ def _run_cmd(self, cmd, err_msg=None, tolerate_err=False, stdin=None): | |||
| out, err = p.communicate(input=stdin) | |||
|
|
|||
| if p.returncode != 0 and not tolerate_err: | |||
| LOG.error("Command {0} failed with {1}".format(cmd, err)) | |||
| LOG.error("Command {0} failed with the following error:".format(cmd)) | |||
There was a problem hiding this comment.
Without this change, error can already be shown in multiple lines if it includes "\n":
err = "multiline\nerror\nmessage"
2023-08-01 02:01:30 +0000 [ERROR ] Command skopeo copy --all docker://registry-proxy.engineering.redhat.com/rh-osbs/rhel8-redis-5:rhel-8.6.0-z-containers-candidate-36737-20220620193816 docker://quay.io/redhat-dev2/test----repo1:121 --format v2s2 failed with multiline
error
message
Does error really have "\n"? Seems only stdout has it.
There was a problem hiding this comment.
You're right, looks like splitting by \n is not the correct solution for local executor. I have instead added textwrap that splits the long message based on length.
midnightercz
reviewed
Aug 1, 2023
pubtools/_quay/command_executor.py
Outdated
| LOG.error("Command {0} failed with {1}".format(cmd, err)) | ||
| LOG.error("Command {0} failed with the following error:".format(cmd)) | ||
| for line in err.splitlines(): | ||
| LOG.error(line) |
There was a problem hiding this comment.
Can you add an indentation here? For example " " could be enough to make it more readable in logs
midnightercz
approved these changes
Aug 1, 2023
querti
force-pushed
the
improve-command-logs
branch
2 times, most recently
from
cfbdd66
to
eeaa3c2
Compare
When skopeo copy fails, the error message cannot be accessed because the log of the command output gets truncated. Since the command generates new line characters, use them to split the output and log each line separately.
querti
force-pushed
the
improve-command-logs
branch
from
600ec7e
to
e73b43d
Compare
emilyzheng
approved these changes
Aug 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When skopeo copy fails, the error message cannot be accessed because the log of the command output gets truncated. Since the command generates new line characters, use them to split the output and log each line separately.
Refers to CLOUDDST-19503