remotecache: handle not implemented error for Info()#5257
remotecache: handle not implemented error for Info()#5257tonistiigi merged 1 commit intomoby:masterfrom
Conversation
tonistiigi
force-pushed
the
remotecache-not-implemented-err
branch
from
6d3f356 to
74b0515
Compare
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
curious if matching the interface would've been an alternative, but not sure if we can with the Moby integration.
| for _, d := range r.Descriptors { | ||
| if _, err := r.Provider.Info(ctx, d.Digest); err != nil { | ||
| return "" | ||
| if !cerrdefs.IsNotImplemented(err) { |
There was a problem hiding this comment.
For others; I was curious if this should be using buildkit's or moby's errdefs definitions, but the Provider.Info interface looks to be defined by containerd;
buildkit/vendor/github.com/containerd/containerd/content/content.go
Lines 117 to 123 in bff1d81
(wondering if GoDoc for the interface should also mention NotImplemented, but maybe it's not strictly how containerd expected that to be handled other than "don't implement the interface")
|
OH! Typo in the commit message; perhaps good to fix if you want to be able to find it back; |
Moby graphdriver backend does not implement Info() and such case should not be used as a signal for blob needed to be skipped as it is unavailable. This requires a change in Moby side as well to return a typed error instead of string. Signed-off-by: Tonis Tiigi <[email protected]>
tonistiigi
force-pushed
the
remotecache-not-implemented-err
branch
from
74b0515 to
8fdf84f
Compare
jedevc
changed the title
2 participants
fixes #5242
Moby graphdriver backend does not implement Info() and such case should not be used as a signal for
blob needed to be skipped as it is unavailable.
This requires a change in Moby side as well to return a typed error instead of string.