chunked: downgrade loading cache file msg to info #1906
Conversation
it can happen for any reason, like for example using a new cache file format, in this case the file is recreated with the last version. This is internal only and should not be displayed by default. Closes: containers#1905 Signed-off-by: Giuseppe Scrivano <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe 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 |
There was a problem hiding this comment.
My first impression is that logging io.EOF (if that’s what it is) as an error, as reported in #1905 , indicates a programming bug; that bug should be fixed, not papered over. (io.EOF is a non-failure value that should be handled, perhaps by converting into UnexpectedEOF).
E.g. there already is
if version != cacheVersion {
return nil, nil //nolint: nilnil
}so we shouldn’t be failing on a version change, in principle.
I guess cacheVersion was not bumped recently and that’s the root cause. Oh well, not something we can fix retroactively.
The API of binary.Read is inconvenient for us, in that it returns io.EOF if the EOF falls on a value boundary. I suppose it would be easiest to wrap it into something that converts io.EOF into UnexpectedEOF.
Alternatively, the use of the io.Reader abstraction over bigDataBuffer could be eliminated entirely, but dealing with byte indices manually would be tedious.
Decreasing the severity of the warning LGTM either way — still, typically we shouldn’t be triggering that log line at all.
|
/lgtm |
it can happen for any reason, like for example using a new cache file format, in this case the file is recreated with the last version. This is internal only and should not be displayed by default.
Closes: #1905