-
Notifications
You must be signed in to change notification settings - Fork 402
docker: export UnexpectedHTTPStatusError #2801
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
Conversation
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.
Thanks! The API makes sense.
To be universal: There is also getExternalBlob
(though I’m unsure what exactly to do there), getOneSignature
, and httpResponseToError
. At least the last one is invoked on ~every operation, as one of the first requests, so it might frequently be the one that actually reports a failure.
Some callers might like to make decisions based on the http server error that was returned. In particular we would like c/common/pkg/retry to match this error so it can retry image pulls/pushes on 5XX errors as they seems to be a quite common problem[1]. [1] containers/common#2299 Signed-off-by: Paul Holzinger <pholzing@redhat.com>
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, please merge (or ping me) whenever appropriate.
(I wouldn’t mind a newUnexpectedHTTPStatusError(*http.Response)
helper, but this is simple enough, and I don’t expect many more instances to be added in the future.)
Add a small helper to create the error directly from the http response without having to set the struct fields at each caller. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
So that callers can actually check the status code of all requests if needed. This changes error text slightly but I think it still carries the same meaning. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Added the new helper function as well |
Some callers might like to make decisions based on the http server error that was returned.
In particular we would like c/common/pkg/retry to match this error so it can retry image pulls/pushes on 5XX errors as they seems to be a quite common problem[1].
[1] containers/common#2299