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
Revert diff/walking error change #5566
Conversation
Build succeeded.
|
Line 121 in 6ae9090
Does it mean that we need to perform roll-back after a Commit failure?eg. if newReference {
if abortErr := s.store.Abort(ctx, config.Reference); abortErr != nil {
log.G(ctx).WithError(abortErr).WithField("ref", config.Reference).Warnf("failed to delete diff upload")
}
} Lines 111 to 117 in 6ae9090
Since if err := cw.Commit(ctx, 0, dgst, commitopts...); err != nil {
if !errdefs.IsAlreadyExists(err) {
errOpen = err
return errors.Wrap(err, "failed to commit")
}
} |
Clarify error scope and create variable for deferring cleanup Signed-off-by: Derek McGowan <derek@mcg.dev>
ed56170
to
69f43d4
Compare
I think in this case calling Abort is the best thing to do. Not aborting on Commit failure allows the caller to try again, but that is not relevant here. I updated the code to just use |
Build succeeded.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thanks for the additional comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Clarify error scope and create variable for deferring cleanup
Reverts #5551