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

Revisit alwaysSucceedWhenCancelled to succeed if error is context cancelled at all times. #5686

Open
tejal29 opened this issue Apr 15, 2021 · 0 comments
Labels
internal kind/todo implementation task/epic for the skaffold team priority/p2 May take a couple of releases

Comments

@tejal29
Copy link
Contributor

tejal29 commented Apr 15, 2021

When refactoring code for #5537, I notice alwaysSucceedWhenCancelled is a little confusing and returns nil only if ctx.Err is cancelled. There are tests in place e.g. here which rely on this behavior.

This bug is to revisit this and remove the extra check in main.

// alwaysSucceedWhenCancelled returns nil if the context was cancelled.
// If the error is due to cancellation, return it as it gets swallowed
// in skaffold main.
// For all other errors, pass through known errors.
// TODO: Return nil if error is `context.Cancelled` and remove check in main.
func alwaysSucceedWhenCancelled(ctx context.Context, runCtx *runcontext.RunContext, err error) error {
	if err == nil {
		return err
	}
	// if the context was cancelled act as if all is well
	if ctx.Err() == context.Canceled {
		return nil
	} else if err == context.Canceled {
		return err
	}
	return sErrors.ShowAIError(runCtx, err)
}
@tejal29 tejal29 added internal kind/todo implementation task/epic for the skaffold team priority/p2 May take a couple of releases labels Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal kind/todo implementation task/epic for the skaffold team priority/p2 May take a couple of releases
Projects
None yet
Development

No branches or pull requests

1 participant