libct/error.go: rm ConfigError (alt)#3176
Merged
AkihiroSuda merged 2 commits intoopencontainers:masterfrom Sep 2, 2021
Merged
Conversation
kolyshkin
commented
Aug 24, 2021
| for _, c := range warns { | ||
| if err := c(config); err != nil { | ||
| logrus.WithError(err).Warnf("invalid configuration") | ||
| logrus.WithError(err).Warn("invalid configuration") |
Contributor
Author
There was a problem hiding this comment.
Technically it doesn't belong here; let me remove it.
Contributor
Author
There was a problem hiding this comment.
moved to the first commit
All the errors returned from Validate should tell about a configuration error. Some were lacking a context, so add it. While at it, fix abusing fmt.Errorf and logrus.Warnf where the argument do not contain %-style formatting. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
ConfigError was added by commit e918d02, while removing runc own error system, to preserve a way for a libcontainer user to distinguish between a configuration error and something else. The way ConfigError is implemented requires a different type of check (compared to all other errors defined by error.go). An attempt was made to rectify this, but the resulting code became even more complicated. As no one is using this functionality (of differentiating a "bad config" type of error from other errors), let's just drop the ConfigError type. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
rm-config-error-alt
branch
from
1ea77a6 to
538ba84
Compare
Contributor
Author
|
Setting 1.1.0 milestone as I'd like this refactoring (which changes libcontainer API) to be over before 1.1.0 one way or another. |
AkihiroSuda
approved these changes
Sep 2, 2021
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
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.
This is the alternative to #3056, addressing review comment from #3033 (comment).
ConfigErrortype was added by commit e918d02, while removing runc'sown error system, to preserve a way for a libcontainer user to distinguish
between a configuration error and something else.
The way ConfigError is implemented requires a different type of check
(compared to all other errors defined by error.go). An attempt was made
to rectify this, but the resulting code became even more complicated.
As no one is using this functionality (of differentiating a "bad config"
type of error from other errors), let's just drop the ConfigError type.