bugfix: pull image before run and upgrade #1419
Conversation
|
We found this is your first time to contribute to Pouch, @wrfly |
Codecov Report
@@ Coverage Diff @@
## master #1419 +/- ##
==========================================
- Coverage 38.77% 33.71% -5.06%
==========================================
Files 250 254 +4
Lines 16629 18759 +2130
==========================================
- Hits 6448 6325 -123
- Misses 9355 11610 +2255
+ Partials 826 824 -2
|
cli/pull.go
Outdated
| } | ||
| namedRef = reference.TrimTagForDigest(reference.WithDefaultTagIfMissing(namedRef)) | ||
|
|
||
| // the error comes from daemon/mgr/image_store.go |
There was a problem hiding this comment.
Hi, @wrfly , I think we can use assert to check the error is RespError or not. If the statusCode is 404, we can continue to pull the image. It seems that it's better than using string match. WDYT?
BTW, we cannot use the errtypes to check the error is notFound or other type since this error comes from the http or rpc call, which has been marshaled and unmarshaled during the network. But we have the code for the status error.
There was a problem hiding this comment.
Yeah, assert is better, I just use it in testing before. I'll change it later.
There was a problem hiding this comment.
但是返回的 inspectError 就是一个单纯的error呀，没有 statusCode， CommonAPIClient 把 resp.StatusCode 吃掉了
// ImageInspect requests daemon to inspect an image.
func (client *APIClient) ImageInspect(ctx context.Context, name string) (types.ImageInfo, error) {
image := types.ImageInfo{}
resp, err := client.get(ctx, "/images/"+name+"/json", nil, nil)
if err != nil {
return image, err
}
defer ensureCloseReader(resp)
err = decodeBody(&image, resp.Body)
return image, err
}所以现在的想法是用assert来判断返回的error里面有没有 not found 字样（很ugly）
然后，不明白调用APIClient的时候返回的是 {"message": "xxxxxx"} 这样的一种error形式， 也许可以用 {"code": xxx, "message": "xxx"}，不然一个单纯的 error 只能用字符串匹配了。。
There was a problem hiding this comment.
Hi @wrfly , please check the client.get method and it will return RespError.
you can use the following statement to check the error:
if eErr, ok := err.(client.RespError); ok {
// we need to add StatusCode method.
if e.Err.StatusCode() != 404 {
}
}
Does it make senses to you?
@wrfly please try to use English. :) Try your best
There was a problem hiding this comment.
Yeah, understood.
I thought it's ugly to convert errors around at first, but I saw moby use it too. It's my problem that I haven't see this usage before.
There was a problem hiding this comment.
OK. So please update your code 😄
cli/pull.go
Outdated
| } | ||
| } | ||
|
|
||
| namedRef, err := reference.Parse(image) |
There was a problem hiding this comment.
I think we can put the parse job after the inspect call. The error check doesn't need to involve the reference check if we use the status code to check error.
There was a problem hiding this comment.
Sorry, I don't understand you. 可以用中文的😂
There was a problem hiding this comment.
I means the put the reference.Parse statement after the 404 check.
Signed-off-by: wrfly <mr.wrfly@gmail.com>
Signed-off-by: wrfly <mr.wrfly@gmail.com>
|
LGTM @wrfly great job. 👍 |
|
@wrfly Could you please to rebase your commits into one commit? Thanks |
|
Actually I think we could help to squash this when merging. @fuweid |
fix #1379
and remove the duplicate image tag in logs