libimage: fix one copyToStorage error message #2390
Conversation
ImageReference is an interface and we generally have no idea about the
underlying layout here and if that can be printed as string. In case of
the docker transport we get:
{{{docker.io library/busybox} latest} %!s(bool=false)}
That is clearly wrong and confusing, instead use transports.ImageName()
which is the recommended way to refer to images in the UI.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
cc @mtrmac |
|
I wonder why none of the linters complained about this, the interface does not implement |
|
LGTM |
| _, err := c.copyInternal(ctx, source, destination, &resolvedReference) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("internal error: unable to copy from source %s: %w", source, err) | ||
| return nil, fmt.Errorf("internal error: unable to copy from source %s: %w", transports.ImageName(source), err) |
There was a problem hiding this comment.
@flouthoc Also, this should not say “internal error”. Maybe this should not be wrapping the error at all — copyInternal(…, nil) should, conceptually, be producing a valid error.
Before this PR, I see
Error: internal error: unable to copy from source {{{localhost:5000 pushed} latest} %!s(bool=false)}: initializing source docker://localhost:5000/pushed:latest: pinging container registry localhost:5000: Get "https://localhost:5000/v2/": http: server gave HTTP response to HTTPS client
There was a problem hiding this comment.
(The http:… part is ~expected on that run, and does not need investigating.)
There was a problem hiding this comment.
I will open a followup PR to correct this.
4 participants
ImageReference is an interface and we generally have no idea about the underlying layout here and if that can be printed as string. In case of the docker transport we get:
{{{docker.io library/busybox} latest} %!s(bool=false)}That is clearly wrong and confusing, instead use transports.ImageName() which is the recommended way to refer to images in the UI.