Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add retry when commiting a manifest #1041

Merged
merged 1 commit into from
Jun 7, 2021
Merged

Add retry when commiting a manifest #1041

merged 1 commit into from
Jun 7, 2021

Conversation

DennisDenuto
Copy link
Contributor

@DennisDenuto DennisDenuto commented Jun 7, 2021

  • only retry on certain 'temporary' http status codes
  • add tests to ensure retries do not occur on 'non-temp' http status
    codes. And to make sure progress bar still works when retrying

Attempts to fix: #1038

Note:
Running hack/presubmit.sh resulted in the following:

~/workspace/go-containerregistry/pkg/authn/k8schain ~/workspace/go-containerregistry ~/workspace/go-containerregistry
go: github.com/google/go-containerregistry@v0.5.2-0.20210601193515-0ffa4a5c8691 requires
        cloud.google.com/go@v0.83.0: missing go.sum entry; to add it:
        go mod download cloud.google.com/go
~/workspace/go-containerregistry ~/workspace/go-containerregistry

This seems to me unrelated to the change in the PR... tried to go mod download cloud.google.com/go but it still complains.

Authored-by: Dennis Leon leonde@vmware.com

@jonjohnsonjr
Copy link
Collaborator

    cloud.google.com/go@v0.83.0: missing go.sum entry; to add it:

@imjasonh any idea?

pkg/v1/remote/write.go Outdated Show resolved Hide resolved
pkg/v1/remote/write.go Outdated Show resolved Hide resolved
- only retry on certain 'temporary' http status codes
- add tests to ensure retries do not occur on 'non-temp' http status
codes. And to make sure progress bar still works when retrying

Authored-by: Dennis Leon <leonde@vmware.com>
@codecov-commenter
Copy link

Codecov Report

Merging #1041 (317b3f8) into main (b69114f) will increase coverage by 0.10%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1041      +/-   ##
==========================================
+ Coverage   75.04%   75.15%   +0.10%     
==========================================
  Files         107      107              
  Lines        5074     5071       -3     
==========================================
+ Hits         3808     3811       +3     
+ Misses        720      717       -3     
+ Partials      546      543       -3     
Impacted Files Coverage Δ
pkg/v1/remote/write.go 63.59% <75.00%> (+0.20%) ⬆️
pkg/v1/remote/multi_write.go 58.95% <0.00%> (+2.98%) ⬆️

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 b69114f...317b3f8. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic when retrying MultiWrite with the same regv1.Update channel
3 participants