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

fix: Retry for http timeout error. Fixes #9271 #9335

Merged
merged 1 commit into from
Aug 15, 2022

Conversation

jingkkkkai
Copy link
Contributor

Signed-off-by: Jingkai Hung [email protected]

Fixes #9271

@@ -334,3 +335,7 @@ func (g *ArtifactDriver) ListObjects(artifact *wfv1.Artifact) ([]string, error)
func (g *ArtifactDriver) IsDirectory(artifact *wfv1.Artifact) (bool, error) {
return false, errors.New(errors.CodeNotImplemented, "IsDirectory currently unimplemented for GCS")
}

func isTransientErr(err error) bool {
return http.IsHttpTimeoutErr(err) || isTransientGCSErr(err)
Copy link
Member

Choose a reason for hiding this comment

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

Why not adding http.IsHttpTimeoutErr to the existing isTransientGCSErr?

Copy link
Member

Choose a reason for hiding this comment

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

There's also already isTransientError in the codebase. Consider reusing it.

@jingkkkkai jingkkkkai force-pushed the retry-for-http-timeout branch 4 times, most recently from f030fe7 to 08da828 Compare August 15, 2022 19:33
@terrytangyuan terrytangyuan enabled auto-merge (squash) August 15, 2022 19:37
@terrytangyuan terrytangyuan merged commit 12466b7 into argoproj:master Aug 15, 2022
juchaosong pushed a commit to juchaosong/argo-workflows that referenced this pull request Nov 3, 2022
Signed-off-by: Jingkai Hung <[email protected]>

Signed-off-by: Jingkai Hung <[email protected]>
Signed-off-by: juchao <[email protected]>
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.

None yet

2 participants