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

Use API server retry logic #6538

Merged
merged 1 commit into from Nov 4, 2016
Merged

Conversation

dooferlad
Copy link
Contributor

When connecting to the API server the retry logic didn't fire, which resulted in all the workers restarting after three seconds. The intent was obviously different. Will now try connecting every 200ms for a second. A pleasing side effect is that tests run much more quickly without 3 second pauses in them.

Tested by running the unit tests.

…resulted in all the workers restarting after three seconds. The intent was obviously different. Will now try connecting every 200ms for a second. A pleasing side effect is that tests run much more quickly without 3 second pauses in them.
@dooferlad dooferlad changed the base branch from staging to develop November 3, 2016 15:19
@dooferlad dooferlad changed the title Use API server the retry logic Use API server retry logic Nov 3, 2016
@bz2
Copy link
Contributor

bz2 commented Nov 3, 2016

!!retry!!

2 similar comments
@nskaggs
Copy link
Contributor

nskaggs commented Nov 3, 2016

!!retry!!

@dooferlad
Copy link
Contributor Author

!!retry!!

@dooferlad
Copy link
Contributor Author

@bz2 @nskaggs can the bot provide/link to some help for available commands? Maybe just some text on the Jenkins page that the details link points to if we can't customise the checks have failed/passed message.

@nskaggs
Copy link
Contributor

nskaggs commented Nov 4, 2016

Anything between !! !! Will trigger a build. It seems like @gz, the bot doesn't like you :(

@nskaggs
Copy link
Contributor

nskaggs commented Nov 4, 2016

!!weneedaretrythebotlikesme!!

@dooferlad
Copy link
Contributor Author

@nskaggs boo. The quick tests seem to be broken too :-(

@nskaggs
Copy link
Contributor

nskaggs commented Nov 4, 2016

@dooferlad yea, looks like the upgrade just broke things.

@nskaggs
Copy link
Contributor

nskaggs commented Nov 4, 2016

!!build!!

Copy link
Contributor

@kat-co kat-co left a comment

Choose a reason for hiding this comment

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

LGTM. Note: we might want to tweak this again as we may be standardizing on https://gopkg.in/retry.v1


if openAttempt.Min == 0 && openAttempt.Delay > 0 {
openAttempt.Min = int(openAttempt.Total / openAttempt.Delay)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would instead fail here. This is a great way to hide issues in calling code.

@dooferlad
Copy link
Contributor Author

$$merge$$

@jujubot
Copy link
Collaborator

jujubot commented Nov 4, 2016

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 9f44391 into juju:develop Nov 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants