Skip to content

Update handling of registry errors#3173

Merged
dmcgowan merged 1 commit intocontainerd:masterfrom
crosbymichael:reg-errors
Apr 4, 2019
Merged

Update handling of registry errors#3173
dmcgowan merged 1 commit intocontainerd:masterfrom
crosbymichael:reg-errors

Conversation

@crosbymichael
Copy link
Copy Markdown
Member

@crosbymichael crosbymichael commented Apr 4, 2019

Update to #3109

Signed-off-by: Michael Crosby [email protected]

estesp
estesp reviewed Apr 4, 2019
View reviewed changes
@crosbymichael
Update handling of registry errors
de1da8b 
Update to containerd#3109

Signed-off-by: Michael Crosby <[email protected]>
@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 4, 2019

Codecov Report

Merging #3173 into master will increase coverage by 4.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3173      +/-   ##
==========================================
+ Coverage   45.24%   49.26%   +4.02%     
==========================================
  Files         111      100      -11     
  Lines       11978     9426    -2552     
==========================================
- Hits         5419     4644     -775     
+ Misses       5725     3957    -1768     
+ Partials      834      825       -9
Flag Coverage Δ
#linux 49.26% <100%> (-0.02%) ⬇️
#windows ?
Impacted Files Coverage Δ
remotes/docker/fetcher.go 61.53% <100%> (-4.12%) ⬇️
remotes/docker/auth.go 63.82% <0%> (-3.97%) ⬇️
remotes/docker/status.go 21.42% <0%> (-3.58%) ⬇️
platforms/cpuinfo.go 3.77% <0%> (-0.85%) ⬇️
log/context.go 36.84% <0%> (-0.66%) ⬇️
namespaces/context.go 55% <0%> (-0.56%) ⬇️
errdefs/grpc.go 74.6% <0%> (-0.4%) ⬇️
remotes/docker/pusher.go 0% <0%> (ø) ⬆️
content/local/locks.go 100% <0%> (ø) ⬆️
... and 62 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4edc733...de1da8b. Read the comment docs.

estesp
estesp approved these changes Apr 4, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Apr 4, 2019

LGTM

However I think the Wrapf approach is probably better. Please test that and open a PR to do that if you find it works. When you output (.Error()) with wrap it will also add the error messages of the wrapped error at the end just like done here, however it also allows the caller to get the typed error through Cause.

@dmcgowan dmcgowan merged commit a7e30fa into containerd:master Apr 4, 2019
@bainsy88
Copy link
Copy Markdown
Contributor

bainsy88 commented Apr 5, 2019

Just catching up on this, will test with Wrapf next week when I’m back in the office. Thanks for the tidy up on the original @crosbymichael

@thaJeztah thaJeztah mentioned this pull request Oct 29, 2020
Closed
@estesp estesp mentioned this pull request Oct 29, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@crosbymichael @codecov-io @dmcgowan @bainsy88 @estesp