Update CI dependencies #141
Conversation
|
I think #131 has similar changes |
|
@bobh66 Oh sorry, I didn't notice the other PR. I'd be happy to merge yours if you can incorporate the few additional things this PR does. |
|
@muvaf - Sure, that's fine - I'll compare them tonight and add the deltas into 131. Thanks! |
|
@bobh66 Actually, I'll merge your PR and then rebase this on top of your changes. Does that sound good? |
|
@muvaf - that's sounds good - I compared the two PRs and other than the workflow updates I think they are essentially the same. Thanks! |
muvaf
changed the title
Signed-off-by: Muvaffak Onus <me@muvaf.com>
|
@turkenh ready to go. |
| @@ -83,7 +87,7 @@ func main() { | |||
| kingpin.FatalIfError(apis.AddToScheme(mgr.GetScheme()), "Cannot add Helm APIs to scheme") | |||
| o := controller.Options{ | |||
| Logger: log, | |||
| MaxConcurrentReconciles: *maxReconcileRate, | |||
| MaxConcurrentReconciles: maxConcurrency, | |||
There was a problem hiding this comment.
Just curious - why hardcode this instead of using the value from the --max-reconcile-rate input parameter?
There was a problem hiding this comment.
Max reconciliation rate caps the number of reconciliation that's allowed to happen at a given time, like 10 per second, whereas max concurrent reconciles controls how many reconciliations can be actively running in parallel at the same time, i.e. don't process new events if there are 10 other go routines processing reconciles. So, we can have another argument to control that but using the same argument for both is sort of misleading and the original number was 10, so I kept it same.
|
Thanks @muvaf - I thought that was strange but that's how provider-template was done so I assumed there was a reason for it. So concurrency is max number of running go routines, and reconciliation rate is max number of outstanding API calls to the server allowed by the RateLimiter? Is it possible for a goroutine to have more than one API call outstanding at any one time? I'm just wondering why they were tied together initially. If the concurrency value is a de-facto limit on the number of outstanding API calls then it makes sense to use the same number for both. But if it's possible to have more outstanding API calls than goroutines, it would be better to separate them. Thanks! |
Description of your changes
This will let provider-helm cap the wait time in error cases to 1 minute.
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Manually.