Merge lp:~chipaca/ubuntu-push/remove-handleconnstate into lp:ubuntu-push/automatic

[John Lenton]

Make client's loop call its session's HasConnectivity directly instead of through handleConnState.

[Samuele Pedroni (pedronis)]

I see, what I was wondering is passing in the channel connCh directly? anyway looks good

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~chipaca/ubuntu-push/remove-handleconnstate into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

scripts/deps.sh ubuntu-push-client.go
scripts/deps.sh server/dev/server.go
scripts/deps.sh server/acceptance/cmd/acceptanceclient.go
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -t launchpad.net/ubuntu-push launchpad.net/ubuntu-push/accounts launchpad.net/ubuntu-push/bus launchpad.net/ubuntu-push/bus/accounts launchpad.net/ubuntu-push/bus/connectivity launchpad.net/ubuntu-push/bus/emblemcounter launchpad.net/ubuntu-push/bus/haptic launchpad.net/ubuntu-push/bus/networkmanager launchpad.net/ubuntu-push/bus/notifications launchpad.net/ubuntu-push/bus/polld launchpad.net/ubuntu-push/bus/powerd launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/unitygreeter launchpad.net/ubuntu-push/bus/windowstack launchpad.net/ubuntu-push/click launchpad.net/ubuntu-push/click/cappinfo launchpad.net/ubuntu-push/click/cblacklist launchpad.net/ubuntu-push/click/cclick launchpad.net/ubuntu-push/click/testing launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/service launchpad.net/ubuntu-push/client/session launchpad.net/ubuntu-push/client/session/seenstate launchpad.net/ubuntu-push/config launchpad.net/ubuntu-push/external/murmur3 launchpad.net/ubuntu-push/identifier launchpad.net/ubuntu-push/identifier/testing launchpad.net/ubuntu-push/launch_helper launchpad.net/ubuntu-push/launch_helper/cual launchpad.net/ubuntu-push/launch_helper/helper_finder launchpad.net/ubuntu-push/launch_helper/legacy launchpad.net/ubuntu-push/logger launchpad.net/ubuntu-push/messaging launchpad.net/ubuntu-push/messaging/cmessaging launchpad.net/ubuntu-push/messaging/reply launchpad.net/ubuntu-push/nih launchpad.net/ubuntu-push/nih/cnih launchpad.net/ubuntu-push/poller launchpad.net/ubuntu-push/protocol launchpad.net/ubuntu-push/server launchpad.net/ubuntu-push/server/api launchpad.net/ubuntu-push/server/broker launchpad.net/ubuntu-push/server/broker/simple launchpad.net/ubuntu-push/server/broker/testing launchpad.net/ubuntu-push/server/broker/testsuite launchpad.net/ubuntu-push/server/dev launchpad.net/ubuntu-push/server/listener launchpad.net/ubuntu-push/server/session launchpad.net/ubuntu-push/server/store launchpad.net/ubuntu-push/sounds launchpad.net/ubuntu-push/testing launchpad.net/ubuntu-push/testing/condition launchpad.net/ubuntu-push/urldispatcher launchpad.net/ubuntu-push/urldispatcher/curldispatcher launchpad.net/ubuntu-push/util launchpad.net/ubuntu-push/server/acceptance/cmd/ launchpad.net/ubuntu-push/server/dev/ launchpad.net/ubuntu-push/ 2>/dev/null | cat > dependencies.tsv
rm -f -r /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 code.google.com/p/go-uuid/uuid
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -u dependencies.tsv
g...

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~chipaca/ubuntu-push/remove-handleconnstate into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

scripts/deps.sh ubuntu-push-client.go
scripts/deps.sh server/dev/server.go
scripts/deps.sh server/acceptance/cmd/acceptanceclient.go
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -t launchpad.net/ubuntu-push launchpad.net/ubuntu-push/accounts launchpad.net/ubuntu-push/bus launchpad.net/ubuntu-push/bus/accounts launchpad.net/ubuntu-push/bus/connectivity launchpad.net/ubuntu-push/bus/emblemcounter launchpad.net/ubuntu-push/bus/haptic launchpad.net/ubuntu-push/bus/networkmanager launchpad.net/ubuntu-push/bus/notifications launchpad.net/ubuntu-push/bus/polld launchpad.net/ubuntu-push/bus/powerd launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/unitygreeter launchpad.net/ubuntu-push/bus/windowstack launchpad.net/ubuntu-push/click launchpad.net/ubuntu-push/click/cappinfo launchpad.net/ubuntu-push/click/cblacklist launchpad.net/ubuntu-push/click/cclick launchpad.net/ubuntu-push/click/testing launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/service launchpad.net/ubuntu-push/client/session launchpad.net/ubuntu-push/client/session/seenstate launchpad.net/ubuntu-push/config launchpad.net/ubuntu-push/external/murmur3 launchpad.net/ubuntu-push/identifier launchpad.net/ubuntu-push/identifier/testing launchpad.net/ubuntu-push/launch_helper launchpad.net/ubuntu-push/launch_helper/cual launchpad.net/ubuntu-push/launch_helper/helper_finder launchpad.net/ubuntu-push/launch_helper/legacy launchpad.net/ubuntu-push/logger launchpad.net/ubuntu-push/messaging launchpad.net/ubuntu-push/messaging/cmessaging launchpad.net/ubuntu-push/messaging/reply launchpad.net/ubuntu-push/nih launchpad.net/ubuntu-push/nih/cnih launchpad.net/ubuntu-push/poller launchpad.net/ubuntu-push/protocol launchpad.net/ubuntu-push/server launchpad.net/ubuntu-push/server/api launchpad.net/ubuntu-push/server/broker launchpad.net/ubuntu-push/server/broker/simple launchpad.net/ubuntu-push/server/broker/testing launchpad.net/ubuntu-push/server/broker/testsuite launchpad.net/ubuntu-push/server/dev launchpad.net/ubuntu-push/server/listener launchpad.net/ubuntu-push/server/session launchpad.net/ubuntu-push/server/store launchpad.net/ubuntu-push/sounds launchpad.net/ubuntu-push/testing launchpad.net/ubuntu-push/testing/condition launchpad.net/ubuntu-push/urldispatcher launchpad.net/ubuntu-push/urldispatcher/curldispatcher launchpad.net/ubuntu-push/util launchpad.net/ubuntu-push/server/acceptance/cmd/ launchpad.net/ubuntu-push/server/dev/ launchpad.net/ubuntu-push/ 2>/dev/null | cat > dependencies.tsv
rm -f -r /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 code.google.com/p/go-uuid/uuid
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -u dependencies.tsv
g...

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~chipaca/ubuntu-push/remove-handleconnstate into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

scripts/deps.sh ubuntu-push-client.go
scripts/deps.sh server/dev/server.go
scripts/deps.sh server/acceptance/cmd/acceptanceclient.go
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -t launchpad.net/ubuntu-push launchpad.net/ubuntu-push/accounts launchpad.net/ubuntu-push/bus launchpad.net/ubuntu-push/bus/accounts launchpad.net/ubuntu-push/bus/connectivity launchpad.net/ubuntu-push/bus/emblemcounter launchpad.net/ubuntu-push/bus/haptic launchpad.net/ubuntu-push/bus/networkmanager launchpad.net/ubuntu-push/bus/notifications launchpad.net/ubuntu-push/bus/polld launchpad.net/ubuntu-push/bus/powerd launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/unitygreeter launchpad.net/ubuntu-push/bus/windowstack launchpad.net/ubuntu-push/click launchpad.net/ubuntu-push/click/cappinfo launchpad.net/ubuntu-push/click/cblacklist launchpad.net/ubuntu-push/click/cclick launchpad.net/ubuntu-push/click/testing launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/service launchpad.net/ubuntu-push/client/session launchpad.net/ubuntu-push/client/session/seenstate launchpad.net/ubuntu-push/config launchpad.net/ubuntu-push/external/murmur3 launchpad.net/ubuntu-push/identifier launchpad.net/ubuntu-push/identifier/testing launchpad.net/ubuntu-push/launch_helper launchpad.net/ubuntu-push/launch_helper/cual launchpad.net/ubuntu-push/launch_helper/helper_finder launchpad.net/ubuntu-push/launch_helper/legacy launchpad.net/ubuntu-push/logger launchpad.net/ubuntu-push/messaging launchpad.net/ubuntu-push/messaging/cmessaging launchpad.net/ubuntu-push/messaging/reply launchpad.net/ubuntu-push/nih launchpad.net/ubuntu-push/nih/cnih launchpad.net/ubuntu-push/poller launchpad.net/ubuntu-push/protocol launchpad.net/ubuntu-push/server launchpad.net/ubuntu-push/server/api launchpad.net/ubuntu-push/server/broker launchpad.net/ubuntu-push/server/broker/simple launchpad.net/ubuntu-push/server/broker/testing launchpad.net/ubuntu-push/server/broker/testsuite launchpad.net/ubuntu-push/server/dev launchpad.net/ubuntu-push/server/listener launchpad.net/ubuntu-push/server/session launchpad.net/ubuntu-push/server/store launchpad.net/ubuntu-push/sounds launchpad.net/ubuntu-push/testing launchpad.net/ubuntu-push/testing/condition launchpad.net/ubuntu-push/urldispatcher launchpad.net/ubuntu-push/urldispatcher/curldispatcher launchpad.net/ubuntu-push/util launchpad.net/ubuntu-push/server/acceptance/cmd/ launchpad.net/ubuntu-push/server/dev/ launchpad.net/ubuntu-push/ 2>/dev/null | cat > dependencies.tsv
rm -f -r /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 code.google.com/p/go-uuid/uuid
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -u dependencies.tsv
g...

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~chipaca/ubuntu-push/remove-handleconnstate into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

scripts/deps.sh ubuntu-push-client.go
scripts/deps.sh server/dev/server.go
scripts/deps.sh server/acceptance/cmd/acceptanceclient.go
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -t launchpad.net/ubuntu-push launchpad.net/ubuntu-push/accounts launchpad.net/ubuntu-push/bus launchpad.net/ubuntu-push/bus/accounts launchpad.net/ubuntu-push/bus/connectivity launchpad.net/ubuntu-push/bus/emblemcounter launchpad.net/ubuntu-push/bus/haptic launchpad.net/ubuntu-push/bus/networkmanager launchpad.net/ubuntu-push/bus/notifications launchpad.net/ubuntu-push/bus/polld launchpad.net/ubuntu-push/bus/powerd launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/unitygreeter launchpad.net/ubuntu-push/bus/windowstack launchpad.net/ubuntu-push/click launchpad.net/ubuntu-push/click/cappinfo launchpad.net/ubuntu-push/click/cblacklist launchpad.net/ubuntu-push/click/cclick launchpad.net/ubuntu-push/click/testing launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/service launchpad.net/ubuntu-push/client/session launchpad.net/ubuntu-push/client/session/seenstate launchpad.net/ubuntu-push/config launchpad.net/ubuntu-push/external/murmur3 launchpad.net/ubuntu-push/identifier launchpad.net/ubuntu-push/identifier/testing launchpad.net/ubuntu-push/launch_helper launchpad.net/ubuntu-push/launch_helper/cual launchpad.net/ubuntu-push/launch_helper/helper_finder launchpad.net/ubuntu-push/launch_helper/legacy launchpad.net/ubuntu-push/logger launchpad.net/ubuntu-push/messaging launchpad.net/ubuntu-push/messaging/cmessaging launchpad.net/ubuntu-push/messaging/reply launchpad.net/ubuntu-push/nih launchpad.net/ubuntu-push/nih/cnih launchpad.net/ubuntu-push/poller launchpad.net/ubuntu-push/protocol launchpad.net/ubuntu-push/server launchpad.net/ubuntu-push/server/api launchpad.net/ubuntu-push/server/broker launchpad.net/ubuntu-push/server/broker/simple launchpad.net/ubuntu-push/server/broker/testing launchpad.net/ubuntu-push/server/broker/testsuite launchpad.net/ubuntu-push/server/dev launchpad.net/ubuntu-push/server/listener launchpad.net/ubuntu-push/server/session launchpad.net/ubuntu-push/server/store launchpad.net/ubuntu-push/sounds launchpad.net/ubuntu-push/testing launchpad.net/ubuntu-push/testing/condition launchpad.net/ubuntu-push/urldispatcher launchpad.net/ubuntu-push/urldispatcher/curldispatcher launchpad.net/ubuntu-push/util launchpad.net/ubuntu-push/server/acceptance/cmd/ launchpad.net/ubuntu-push/server/dev/ launchpad.net/ubuntu-push/ 2>/dev/null | cat > dependencies.tsv
rm -f -r /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 code.google.com/p/go-uuid/uuid
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -u dependencies.tsv
g...

[lp:~chipaca/ubuntu-push/remove-handleconnstate]

383. By John Lenton on 2015-03-18

bump the timeout

=== modified file 'client/client.go'
--- client/client.go	2015-03-10 12:32:36 +0000
+++ client/client.go	2015-03-18 16:34:26 +0000
@@ -382,13 +382,6 @@
 	}
 }
 
-// handleConnState deals with connectivity events
-func (client *PushClient) handleConnState(hasConnectivity bool) {
-	client.log.Debugf("handleConnState: %v", hasConnectivity)
-	client.session.HasConnectivity(hasConnectivity)
-	client.log.Debugf("handled.")
-}
-
 // filterBroadcastNotification finds out if the notification is about an actual
 // upgrade for the device. It expects msg.Decoded entries to look
 // like:
@@ -500,7 +493,7 @@
 
 // Loop calls doLoop with the "real" handlers
 func (client *PushClient) Loop() {
-	client.doLoop(client.handleConnState,
+	client.doLoop(client.session.HasConnectivity,
 		client.handleBroadcastNotification,
 		client.handleUnicastNotification,
 		client.handleUnregister,

=== modified file 'client/client_test.go'
--- client/client_test.go	2015-03-10 12:32:36 +0000
+++ client/client_test.go	2015-03-18 16:34:26 +0000
@@ -532,7 +532,7 @@
 func (s *derivePollerSession) Close()                            {}
 func (s *derivePollerSession) ClearCookie()                      {}
 func (s *derivePollerSession) State() session.ClientSessionState { return session.Unknown }
-func (s *derivePollerSession) HasConnectivity(bool) error        { return nil }
+func (s *derivePollerSession) HasConnectivity(bool)              {}
 func (s *derivePollerSession) KeepConnection() error             { return nil }
 func (s *derivePollerSession) StopKeepConnection()               {}
 
@@ -1068,9 +1068,9 @@
 		return session.Disconnected
 	}
 }
-func (s *loopSession) HasConnectivity(hasConn bool) error { s.hasConn = hasConn; return nil }
-func (s *loopSession) KeepConnection() error              { return nil }
-func (s *loopSession) StopKeepConnection()                {}
+func (s *loopSession) HasConnectivity(hasConn bool) { s.hasConn = hasConn }
+func (s *loopSession) KeepConnection() error        { return nil }
+func (s *loopSession) StopKeepConnection()          {}
 
 func (cs *clientSuite) TestLoop(c *C) {
 	cli := NewPushClient(cs.configPath, cs.leveldbPath)
@@ -1094,6 +1094,10 @@
 	// at and the loop itself.
 	tick := func() { cli.sessionConnectedCh <- 42 }
 
+	c.Assert(cli.session, NotNil)
+	cli.session.StopKeepConnection()
+	cli.session = &loopSession{}
+
 	go cli.Loop()
 
 	// sessionConnectedCh to nothing in particular, but it'll help sync this test
@@ -1101,10 +1105,6 @@
 	tick()
 	c.Check(cs.log.Captured(), Matches, "(?msi).*Session connected after 42 attempts$")
 
-	c.Assert(cli.session, NotNil)
-	cli.session.StopKeepConnection()
-	cli.session = &loopSession{}
-
 	// loop() should have connected:
 	//  * connCh to the connectivity checker
 	c.Check(cli.session.State(), Equals, session.Disconnected)

=== modified file 'client/session/session.go'
--- client/session/session.go	2015-03-17 16:46:55 +0000
+++ client/session/session.go	2015-03-18 16:34:26 +0000
@@ -131,7 +131,7 @@
 	Close()
 	ClearCookie()
 	State() ClientSessionState
-	HasConnectivity(bool) error
+	HasConnectivity(bool)
 	KeepConnection() error
 	StopKeepConnection()
 }
@@ -777,10 +777,8 @@
 	close(sess.stopCh)
 }
 
-func (sess *clientSession) HasConnectivity(hasConn bool) error {
+func (sess *clientSession) HasConnectivity(hasConn bool) {
 	sess.connCh <- hasConn
-	// XXX throw errors if called from weird state?
-	return nil
 }
 
 func init() {

=== modified file 'launch_helper/kindpool_test.go'
--- launch_helper/kindpool_test.go	2014-08-22 14:54:41 +0000
+++ launch_helper/kindpool_test.go	2015-03-18 16:34:26 +0000
@@ -45,7 +45,7 @@
 	select {
 	case res := <-ch:
 		return res
-	case <-time.After(100 * time.Millisecond):
+	case <-time.After(time.Second):
 		c.Fatal("timeout waiting for result")
 	}
 	return nil