-
Notifications
You must be signed in to change notification settings - Fork 55
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
Remove error logging for deleteFromNetwork call #83
Conversation
@Luap99: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@Luap99: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
See containers/podman#8571 (comment) for a example error output. |
/approve |
@saschagrunert PTAL |
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: Luap99, rhatdan 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 |
LGTM, needs a rebase though |
While working on podman#8571 I noticed that calling `deleteFromNetwork` logs the same error twice. For example it errors if the iptables rules do not exists and this creates a very verbose error. Since the error is returned I do not see any point in logging it. The caller should decide if the error should be logged or not. It is super confusing for users to get a error logged when they have no control to change anything. In case of podman#8571 the user would only want to call this if the iptables rules are flushed, getting a error in this case is super confusing. Signed-off-by: Paul Holzinger <[email protected]>
6128d1e
to
acc88df
Compare
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
/lgtm |
The changes from cri-o/ocicni#83 are needed to improve the user experience when using the new network reload command. see: containers#8571 (comment) Signed-off-by: Paul Holzinger <[email protected]>
While working on containers/podman#8571 I noticed that calling
deleteFromNetwork
logs the same error twice. It errors for example if the iptables rules do
not exists and create a very verbose error. Since the error is returned
I do not see any point in logging it. The caller should decide if the
error should be logged or not.
It is super confusing for users to get a error logged when they have no
control to change anything. In case of containers/podman#8571 the user would only want
to call this if the iptables rules are flushed, getting a error in this case
is super confusing.