feat: make http client retryable #6632
Conversation
yujunz
force-pushed
the
retry
branch
2 times, most recently
from
e08fc46
to
fc65f29
Compare
Codecov Report
@@ Coverage Diff @@
## master #6632 +/- ##
==========================================
- Coverage 41.29% 41.29% -0.01%
==========================================
Files 156 156
Lines 20688 20689 +1
==========================================
Hits 8543 8543
- Misses 10937 10938 +1
Partials 1208 1208
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks for this PR @yujunz.
LGTM and non-intrusive to me, and I think it's a really useful feature to have to improve stability of pipelines. However, I have some usability concern, please see below.
Also, since you are introducing a new dependency, can you please give some information about that and its license? Thanks.
cmd/argocd/commands/root.go
Outdated
| @@ -69,5 +69,6 @@ func NewCommand() *cobra.Command { | |||
| command.PersistentFlags().StringSliceVarP(&clientOpts.Headers, "header", "H", []string{}, "Sets additional header to all requests made by Argo CD CLI. (Can be repeated multiple times to add multiple headers, also supports comma separated headers)") | |||
| command.PersistentFlags().BoolVar(&clientOpts.PortForward, "port-forward", config.GetBoolFlag("port-forward"), "Connect to a random argocd-server port using port forwarding") | |||
| command.PersistentFlags().StringVar(&clientOpts.PortForwardNamespace, "port-forward-namespace", config.GetFlag("port-forward-namespace", ""), "Namespace name which should be used for port forwarding") | |||
| command.PersistentFlags().IntVar(&clientOpts.RetryMax, "retry-max", 0, "Maximum number of retries") | |||
There was a problem hiding this comment.
I don't think retry-max is a good name for the option. The name should imply what is actually retried, although I cannot think of a proper name right now. Maybe http-retries or api-retries would fit better? Also, the description should be adapted to be a little more verbose.
There was a problem hiding this comment.
Renamed to http-retry-max and adapted the description to be more verbose.
The new dependency hashicorp/go-retryablehttp is released under MPL 2.0 which should be compatible to compile into Apache licensed code like argocd if I understand the FAQ correctly.
|
Signed-off-by: Yujun Zhang <zhangyujun@gmail.com>
Closes #6600