Rev 2371: New SmartServer hooks facility. There are two initial hooks documented in file:///home/robertc/source/baz/hpss-hooks/

Robert Collins robertc at robertcollins.net
Sun Mar 25 10:00:27 BST 2007


At file:///home/robertc/source/baz/hpss-hooks/

------------------------------------------------------------
revno: 2371
revision-id: robertc at robertcollins.net-20070325085956-my8jv7cifqzyltyz
parent: pqm at pqm.ubuntu.com-20070321071219-55447700ec71371f
committer: Robert Collins <robertc at robertcollins.net>
branch nick: hpss-hooks
timestamp: Sun 2007-03-25 18:59:56 +1000
message:
  New SmartServer hooks facility. There are two initial hooks documented
  in bzrlib.transport.smart.SmartServerHooks. The two initial hooks allow
  plugins to execute code upon server startup and shutdown.
  (Robert Collins).
  
  SmartServer in standalone mode will now close its listening socket
  when it stops, rather than waiting for garbage collection. This primarily
  fixes test suite hangs when a test tries to connect to a shutdown server.
  It may also help improve behaviour when dealing with a server running
  on a specific port (rather than dynamically assigned ports).
  (Robert Collins)
added:
  bzrlib/hooks.py                hooks.py-20070325015548-ix4np2q0kd8452au-1
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/branch.py               branch.py-20050309040759-e4baf4e0d046576e
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
  bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
  bzrlib/transport/smart.py      ssh.py-20060608202016-c25gvf1ob7ypbus6-1
=== added file 'bzrlib/hooks.py'
--- a/bzrlib/hooks.py	1970-01-01 00:00:00 +0000
+++ b/bzrlib/hooks.py	2007-03-25 08:59:56 +0000
@@ -0,0 +1,46 @@
+# Copyright (C) 2007 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+
+"""Support for plugin hooking logic."""
+from bzrlib.lazy_import import lazy_import
+lazy_import(globals(), """
+from bzrlib import (
+        errors,
+        )
+""")
+
+
+class Hooks(dict):
+    """A dictionary mapping hook name to a list of callables.
+    
+    e.g. ['FOO'] Is the list of items to be called when the
+    FOO hook is triggered.
+    """
+
+    def install_hook(self, hook_name, a_callable):
+        """Install a_callable in to the hook hook_name.
+
+        :param hook_name: A hook name. See the __init__ method of BranchHooks
+            for the complete list of hooks.
+        :param a_callable: The callable to be invoked when the hook triggers.
+            The exact signature will depend on the hook - see the __init__ 
+            method of BranchHooks for details on each hook.
+        """
+        try:
+            self[hook_name].append(a_callable)
+        except KeyError:
+            raise errors.UnknownHook(self.__class__.__name__, hook_name)

=== modified file 'NEWS'
--- a/NEWS	2007-03-21 04:28:02 +0000
+++ b/NEWS	2007-03-25 08:59:56 +0000
@@ -1,5 +1,20 @@
 IN DEVELOPMENT
 
+  INTERNALS:
+
+    * New SmartServer hooks facility. There are two initial hooks documented
+      in bzrlib.transport.smart.SmartServerHooks. The two initial hooks allow
+      plugins to execute code upon server startup and shutdown.
+      (Robert Collins).
+
+   * SmartServer in standalone mode will now close its listening socket
+     when it stops, rather than waiting for garbage collection. This primarily
+     fixes test suite hangs when a test tries to connect to a shutdown server.
+     It may also help improve behaviour when dealing with a server running
+     on a specific port (rather than dynamically assigned ports).
+     (Robert Collins)
+  
+
 bzr 0.15 (not finalised)
 
   INTERNALS:

=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py	2007-03-13 01:00:34 +0000
+++ b/bzrlib/branch.py	2007-03-25 08:59:56 +0000
@@ -54,6 +54,7 @@
                            NotBranchError, UninitializableFormat,
                            UnlistableStore, UnlistableBranch,
                            )
+from bzrlib.hooks import Hooks
 from bzrlib.symbol_versioning import (deprecated_function,
                                       deprecated_method,
                                       DEPRECATED_PARAMETER,
@@ -884,7 +885,7 @@
             control_files.unlock()
 
 
-class BranchHooks(dict):
+class BranchHooks(Hooks):
     """A dictionary mapping hook name to a list of callables for branch hooks.
     
     e.g. ['set_rh'] Is the list of items to be called when the
@@ -897,7 +898,7 @@
         These are all empty initially, because by default nothing should get
         notified.
         """
-        dict.__init__(self)
+        Hooks.__init__(self)
         # Introduced in 0.15:
         # invoked whenever the revision history has been set
         # with set_revision_history. The api signature is
@@ -936,20 +937,6 @@
         # and an empty branch recieves new_revno of 0, new_revid of None.
         self['post_uncommit'] = []
 
-    def install_hook(self, hook_name, a_callable):
-        """Install a_callable in to the hook hook_name.
-
-        :param hook_name: A hook name. See the __init__ method of BranchHooks
-            for the complete list of hooks.
-        :param a_callable: The callable to be invoked when the hook triggers.
-            The exact signature will depend on the hook - see the __init__ 
-            method of BranchHooks for details on each hook.
-        """
-        try:
-            self[hook_name].append(a_callable)
-        except KeyError:
-            raise errors.UnknownHook('branch', hook_name)
-
 
 # install the default hooks into the Branch class.
 Branch.hooks = BranchHooks()

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2007-03-15 22:35:35 +0000
+++ b/bzrlib/tests/__init__.py	2007-03-25 08:59:56 +0000
@@ -680,7 +680,10 @@
         self._benchcalls = []
         self._benchtime = None
         # prevent hooks affecting tests
-        self._preserved_hooks = bzrlib.branch.Branch.hooks
+        self._preserved_hooks = {
+            bzrlib.branch.Branch:bzrlib.branch.Branch.hooks,
+            bzrlib.transport.smart.SmartTCPServer:bzrlib.transport.smart.SmartTCPServer.hooks,
+            }
         self.addCleanup(self._restoreHooks)
         # this list of hooks must be kept in sync with the defaults
         # in branch.py
@@ -968,7 +971,8 @@
             osutils.set_or_unset_env(name, value)
 
     def _restoreHooks(self):
-        bzrlib.branch.Branch.hooks = self._preserved_hooks
+        for klass, hooks in self._preserved_hooks.items():
+            setattr(klass, 'hooks', hooks)
 
     def tearDown(self):
         self._runCleanups()

=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py	2007-03-12 20:55:23 +0000
+++ b/bzrlib/tests/test_selftest.py	2007-03-25 08:59:56 +0000
@@ -903,6 +903,8 @@
         """The bzrlib hooks should be sanitised by setUp."""
         self.assertEqual(bzrlib.branch.BranchHooks(),
             bzrlib.branch.Branch.hooks)
+        self.assertEqual(bzrlib.transport.smart.SmartServerHooks(),
+            bzrlib.transport.smart.SmartTCPServer.hooks)
 
     def test__gather_lsprof_in_benchmarks(self):
         """When _gather_lsprof_in_benchmarks is on, accumulate profile data.

=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py	2006-11-21 01:08:52 +0000
+++ b/bzrlib/tests/test_smart_transport.py	2007-03-25 08:59:56 +0000
@@ -808,14 +808,46 @@
         self.server = smart.SmartTCPServer(self.backing_transport)
         self.server.start_background_thread()
         self.transport = smart.SmartTCPTransport(self.server.get_url())
+        self.addCleanup(self.tearDownServer)
 
-    def tearDown(self):
+    def tearDownServer(self):
         if getattr(self, 'transport', None):
             self.transport.disconnect()
+            del self.transport
         if getattr(self, 'server', None):
             self.server.stop_background_thread()
-        super(SmartTCPTests, self).tearDown()
-        
+            del self.server
+
+
+class TestServerSocketUsage(SmartTCPTests):
+
+    def test_server_closes_listening_sock_on_shutdown(self):
+        """The server should close its listening socket when its stopped."""
+        self.setUpServer()
+        # clean up the server and initial transport (which wont have connected)
+        server = self.server
+        # force a connection, which uses the listening socket to synchronise
+        # with the server thread, so that when we shut it down it has already
+        # executed the 'self._should_terminate = False' line in the server
+        # method.
+        self.transport.has('.')
+        self.tearDownServer()
+        # make a new connection to break out the inner loop in the server.
+        transport = smart.SmartTCPTransport(server.get_url())
+        # force the connection
+        transport.has('.')
+        # and close it.
+        transport.disconnect()
+        del transport
+        while server._server_thread.isAlive():
+            # this is fugly: we should have an event for the server we can
+            # wait for.
+            import time; time.sleep(0.001)
+        # if the listening socket has closed, we should get a BADFD error
+        # when connecting, rather than a hang.
+        transport = smart.SmartTCPTransport(server.get_url())
+        self.assertRaises(errors.ConnectionError, transport.has, '.')
+
 
 class WritableEndToEndTests(SmartTCPTests):
     """Client to server tests that require a writable transport."""
@@ -901,7 +933,56 @@
         self.setUpServer(readonly=True)
         self.assertRaises(errors.TransportNotPossible, self.transport.mkdir,
             'foo')
-        
+
+
+class TestServerHooks(SmartTCPTests):
+
+    def capture_server_call(self, backing_url, public_url):
+        """Record a server_started|stopped hook firing."""
+        self.hook_calls.append((backing_url, public_url))
+
+    def test_server_started_hook(self):
+        """The server_started hook fires when the server is started."""
+        self.hook_calls = []
+        smart.SmartTCPServer.hooks.install_hook('server_started',
+            self.capture_server_call)
+        self.setUpServer()
+        # at this point, the server will be starting a thread up.
+        # there is no indicator at the moment, so bodge it by doing a request.
+        self.transport.has('.')
+        self.assertEqual([(self.backing_transport.base, self.transport.base)],
+            self.hook_calls)
+
+    def test_server_stopped_hook_simple(self):
+        """The server_stopped hook fires when the server is stopped."""
+        self.hook_calls = []
+        smart.SmartTCPServer.hooks.install_hook('server_stopped',
+            self.capture_server_call)
+        self.setUpServer()
+        result = [(self.backing_transport.base, self.transport.base)]
+        # check the stopping message isn't emitted up front, this also
+        # has the effect of synchronising with the server, so that
+        # when we shut it down it has already executed the 
+        # 'self._should_terminate = False' line in the server method.
+        self.transport.has('.')
+        self.assertEqual([], self.hook_calls)
+        # clean up the server
+        server = self.server
+        self.tearDownServer()
+        # make a new connection to break out the inner loop in the server.
+        transport = smart.SmartTCPTransport(result[0][1])
+        transport.has('.')
+        transport.disconnect()
+        del transport
+        while server._server_thread.isAlive():
+            # this is fugly: we should have an event for the server we can
+            # wait for.
+            import time; time.sleep(0.001)
+        self.assertEqual(result, self.hook_calls)
+
+# TODO: test that when the server suffers an exception that it calls the
+# server-stopped hook.
+
 
 class SmartServerRequestHandlerTests(tests.TestCaseWithTransport):
     """Test that call directly into the handler logic, bypassing the network."""

=== modified file 'bzrlib/transport/smart.py'
--- a/bzrlib/transport/smart.py	2007-03-04 00:28:13 +0000
+++ b/bzrlib/transport/smart.py	2007-03-25 08:59:56 +0000
@@ -212,6 +212,7 @@
     urlutils,
     )
 from bzrlib.bundle.serializer import write_bundle
+from bzrlib.hooks import Hooks
 try:
     from bzrlib.transport import ssh
 except errors.ParamikoNotPresent:
@@ -820,7 +821,10 @@
 
 
 class SmartTCPServer(object):
-    """Listens on a TCP socket and accepts connections from smart clients"""
+    """Listens on a TCP socket and accepts connections from smart clients
+
+    hooks: An instance of SmartServerHooks.
+    """
 
     def __init__(self, backing_transport, host='127.0.0.1', port=0):
         """Construct a new server.
@@ -833,7 +837,8 @@
         """
         self._server_socket = socket.socket()
         self._server_socket.bind((host, port))
-        self.port = self._server_socket.getsockname()[1]
+        self._sockname = self._server_socket.getsockname()
+        self.port = self._sockname[1]
         self._server_socket.listen(1)
         self._server_socket.settimeout(1)
         self.backing_transport = backing_transport
@@ -845,19 +850,30 @@
         from socket import timeout as socket_timeout
         from socket import error as socket_error
         self._should_terminate = False
-        while not self._should_terminate:
+        for hook in SmartTCPServer.hooks['server_started']:
+            hook(self.backing_transport.base, self.get_url())
+        try:
+            while not self._should_terminate:
+                try:
+                    self.accept_and_serve()
+                except socket_timeout:
+                    # just check if we're asked to stop
+                    pass
+                except socket_error, e:
+                    trace.warning("client disconnected: %s", e)
+                    pass
+        finally:
             try:
-                self.accept_and_serve()
-            except socket_timeout:
-                # just check if we're asked to stop
-                pass
-            except socket_error, e:
-                trace.warning("client disconnected: %s", e)
-                pass
+                self._server_socket.close()
+            except socket_error:
+                # ignore errors on close
+                pass
+            for hook in SmartTCPServer.hooks['server_stopped']:
+                hook(self.backing_transport.base, self.get_url())
 
     def get_url(self):
         """Return the url of the server"""
-        return "bzr://%s:%d/" % self._server_socket.getsockname()
+        return "bzr://%s:%d/" % self._sockname
 
     def accept_and_serve(self):
         conn, client_addr = self._server_socket.accept()
@@ -887,6 +903,28 @@
         ## self._server_thread.join()
 
 
+class SmartServerHooks(Hooks):
+    """Hooks for the smart server."""
+
+    def __init__(self):
+        """Create the default hooks.
+
+        These are all empty initially, because by default nothing should get
+        notified.
+        """
+        Hooks.__init__(self)
+        # Introduced in 0.16:
+        # invoked whenever the server starts serving a directory.
+        # The api signature is (backing url, public url).
+        self['server_started'] = []
+        # Introduced in 0.16:
+        # invoked whenever the server stops serving a directory.
+        # The api signature is (backing url, public url).
+        self['server_stopped'] = []
+
+SmartTCPServer.hooks = SmartServerHooks()
+
+
 class SmartTCPServer_for_testing(SmartTCPServer):
     """Server suitable for use by transport tests.
     



More information about the bazaar-commits mailing list