Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
apiclient fix for systems with poor clock resolution #7417
Conversation
| + // Workaround for systems with poor clock resolution on which it's possible | ||
| + // to try to connect 4 times in 0 seconds. | ||
| + if openAttempt.Total == 0 && openAttempt.Delay == 0 { | ||
| + openAttempt.Delay = 1 |
rogpeppe
May 30, 2017
Owner
A delay of 1ns won't help matters too much, I think.
I'd think I'd prefer to fix TestDialAPIMultipleError than at
this logic to the production code - if we want production
code to be able to use the zero DialOpts, I think we should
decide on some semantics, document them, implement them,
and fix the tests that rely on the current semantics.
I'd suggest something like this for the semantics:
// Timeout is the amount of time to wait contacting
// a controller. If this is zero, the Dial may block forever.
Timeout time.Duration
// RetryDelay is the amount of time to wait between
// unsuccessful connection attempts. If this is
// zero, no retries will be made.
RetryDelay time.Duration
rogpeppe
requested changes
May 30, 2017
Thanks, but I'm not sure this is quite the right fix. I've made an alternative suggestion.
|
Fixed the original bug in gopkg.in/retry.v1 |
wupeka
changed the title from
apiclient workaround for systems with poor clock resolution
to
apiclient fix for systems with poor clock resolution
May 31, 2017
rogpeppe
approved these changes
May 31, 2017
LGTM but I think I'd choose a slightly different commit.
| @@ -103,6 +103,6 @@ gopkg.in/macaroon.v1 git ab3940c6c16510a850e1c2dd628b919f0f3f1464 2015-01-21T11: | ||
| gopkg.in/mgo.v2 git f2b6f6c918c452ad107eec89615f074e3bd80e33 2016-08-18T01:52:18Z | ||
| gopkg.in/natefinch/lumberjack.v2 git 514cbda263a734ae8caac038dadf05f8f3f9f738 2016-01-25T11:17:49Z | ||
| gopkg.in/natefinch/npipe.v2 git c1b8fa8bdccecb0b8db834ee0b92fdbcfa606dd6 2016-06-21T03:49:01Z | ||
| -gopkg.in/retry.v1 git c09f6b86ba4d5d2cf5bdf0665364aec9fd4815db 2016-10-25T18:14:30Z | ||
| +gopkg.in/retry.v1 git 8ac863816a3ab5f9a7fc69175e3d0bf3403d6b76 2017-05-31T08:20:38Z |
rogpeppe
May 31, 2017
Owner
Perhaps best to point to retry master HEAD (01631078ef2fdce601e38cfe5f527fab24c9a6d2) ?
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$MERGE$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
|
Build failed: Tests failed |
wupeka commentedMay 30, 2017
•
Edited 1 time
-
wupeka
May 31, 2017
Description of change
Update gopkg.in/retry.v1 with a version that fixes retrying a few times even if we provided '0' seconds as Total time.
QA steps
Run TestDialAPIMultipleError unit test on Windows.