copy: attempt fallback to full retrieval only when possible #2589
Conversation
Signed-off-by: Giuseppe Scrivano <[email protected]>
|
Cc: @Honny1 |
|
|
||
| // ErrFallbackToOrdinaryLayerDownload is a custom error type returned by PutBlobPartial. | ||
| // It suggests to the caller that a fallback mechanism can be used instead of a hard failure. | ||
| type ErrFallbackToOrdinaryLayerDownload struct { |
There was a problem hiding this comment.
Why is this error defined here when a similar error is created in c/storage? Is it okay to have two same-defined types? Which would be used if they have different fields? I'm just curious.
There was a problem hiding this comment.
we don't want the copy package to have a dependency on containers/storage. In this way, it is possible to build containers/image without the storage backend and avoid the vendoring cost
There was a problem hiding this comment.
(Also, it’s not just the technical dependency; c/image/copy strives to contain “generic” code without hard-coding specifics of transports or image formats.
It doesn’t always succeed — we do have some special cases, and really a lot of the private API is specific to the c/storage design and constraints.)
giuseppe
force-pushed
the
partial-pulls-do-not-always-retry
branch
from
a4d7889
to
5cebcbe
Compare
|
@mtrmac thanks for the review. Addressed the comments and pushed a new version |
define a new error type so that the caller can determine whether it is safe to ignore the error and retrieve the resource fully. Signed-off-by: Giuseppe Scrivano <[email protected]>
Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
partial-pulls-do-not-always-retry
branch
from
5cebcbe
to
3dc265d
Compare
thanks! Comments addressed |
Do not attempt the fallback mechanism unless it was reported by the underlying transport.
Follow up for: containers/storage#2118