/juju

api: Export Connection.Ping again #6410

Merged
merged 2 commits into from Oct 10, 2016

Conversation

Reviewers

@howbazaar howbazaar

@wallyworld wallyworld

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@mjs @jujubot @howbazaar @wallyworld
@mjs
Contributor

mjs commented Oct 9, 2016 •
Edited 1 time

... and implement a unit test (it was never tested before).

#6400 removed Ping but this was being used by some external projects.

QA

Full unit test suite run and examined behaviour of a HA failover to ensure no regressions.
@mjs
api: Export Connection.Ping again 
... and implement a unit test (it was never tested before).

#6400/ removed Ping but this was being
used by some external projects.
3f0d2e1
api/interface.go
@@ -178,6 +178,12 @@ type Connection interface {
// All the rest are strange and questionable and deserve extra attention
// and/or discussion.
+ // Something-or-other expects Ping to exist, and *maybe* the heartbeat
@wallyworld

wallyworld Oct 9, 2016

Owner

Can we just say that Ping() is deprecated and to use IsBroken() instead.

@mjs

mjs Oct 9, 2016

Contributor

I will, in the next PR where IsBroken gets added.

@mjs
Contributor

mjs commented Oct 9, 2016

$$merge$$
@jujubot
Contributor

jujubot commented Oct 9, 2016

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

jujubot commented Oct 9, 2016

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/9455
@mjs
apiserver: Fix racy pinger tests 
The pinger tests weren't waiting for the clock when they needed to
leading to the clock sometimes being advanced before it was being waited
on. Additionally, TestAgentConnectionDelaysShutdownWithPing was
unnecessarily complicated having a lot of logic which was only required
before a clock was injected.

Fixes:
https://bugs.launchpad.net/juju/+bug/1625214
https://bugs.launchpad.net/juju/+bug/1627086
f9ee5db
@mjs
Contributor

mjs commented Oct 10, 2016

$$merge$$
@jujubot
Contributor

jujubot commented Oct 10, 2016

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

@jujubot jujubot merged commit de28a82 into juju:master Oct 10, 2016

@mjs mjs deleted the mjs:export-api-conn-ping branch Oct 10, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment