-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
podman load: fix error handling #9677
Conversation
Make sure to properly return loading errors and to set the exit code accordingly. Fixes: containers#9672 Signed-off-by: Valentin Rothberg <[email protected]>
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Merge me |
/lgtm |
# Regression test for #9672 to make sure invalid input yields errors. | ||
invalid=$PODMAN_TMPDIR/invalid | ||
echo "I am an invalid file and should cause a podman-load error" > $invalid | ||
run_podman 125 load -i $invalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wish I'd seen this earlier. In future, I encourage you to check the error message. I'll open a PR to add that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't you be doing yoga on a mountain peek? :)
I didn't add checks since the error messages are ugly. It's something I want to improve soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't you be doing yoga on a mountain peak? :)
What makes you think I'm not? 😉
Ugly or not, the reason for error-message checking is to save yourself from typos or syntax errors:
run_podman load -i $imvalid <--- misspelled variable name, yields "flag needs an argument"
run_podman load -i $nonexistent_path <--- yields "no such file or directory"
...or other mistakes that throw an error for other reasons. (None of those are likely in this case, the code looks good, but an error-message check will greatly reassure future maintainers)
Make sure to properly return loading errors and to set the exit code
accordingly.
Fixes: #9672
Signed-off-by: Valentin Rothberg [email protected]