Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
store: retry once on hashsum mismatches in a Download() #3159
Conversation
mvo5
added some commits
Apr 10, 2017
All checks have passed
All checks have passed
Some checks were not successful
All checks have passed
stolowski
requested changes
Apr 13, 2017
Yes, I think it makes sense, thanks for the change. Just one minor nitpick about error message.
| - // Note that we will retry this way only once. | ||
| - if _, ok := err.(HashError); ok && resume > 0 { | ||
| + // If hashsum is incorrect retry once | ||
| + if _, ok := err.(HashError); ok { | ||
| logger.Debugf("Error on resumed download: %v", err.Error()) |
stolowski
Apr 13, 2017
•
Edited 1 time
-
stolowski
Apr 13, 2017
Contributor
Edited 1 time
-
Apr 13, 2017
I think this error message needs correction now after this change (it's true as long as resume>0 only afaict).
All checks have passed
zyga
merged commit 041b03b
into
snapcore:master
Apr 19, 2017
6 checks passed
continuous-integration/travis-ci/pr
The Travis CI build passed
Details
xenial-amd64
autopkgtest finished (success)
Details
xenial-i386
autopkgtest finished (success)
Details
xenial-ppc64el
autopkgtest finished (success)
Details
yakkety-amd64
autopkgtest finished (success)
Details
zesty-amd64
autopkgtest finished (success)
Details
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
mvo5 commentedApr 10, 2017
Right now we do not retry on hashsum mismatches. However we probably want to retry once in this case. Alternatively we could extend the hashsum error so that it contains a flag if it did an internal retry already with partial-content and only retry in this case.
We are seening errors from the CDN currently (https://forum.snapcraft.io/t/hashsum-failures-during-tests/198) and we suspect that the CDN is sending wrong partial content data.