Merge branch 'master' of github.com:zopefoundation/zc.resumelb

zopefoundation · Mar 3, 2015 · 77c3347 · 77c3347
2 parents b66f8bd + 0ce8459
commit 77c3347
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 20 deletions.
diff --git a/README.rst b/README.rst
@@ -242,6 +242,12 @@ maintaining ZooKeeper trees.
 Change History
 ==============
 
+- Fixed: uncaught applications exceptions were mishandled for HEAD
+  requests.
+
+- Fixed: LB worker paths couldn't be links in single-version mode, or
+  when using alternate pool implementations.
+
 1.0.0 (2015-02-19)
 ------------------
 

diff --git a/src/zc/resumelb/tests.py b/src/zc/resumelb/tests.py
@@ -244,6 +244,44 @@ def workers_generate_500s_for_bad_apps():
     >>> worker.stop()
     """ #"
 
+def workers_generate_500s_for_bad_apps_HEAD():
+    """If an app is poorly behaved and raises exceptions, a worker
+    will generate a 500.
+
+    >>> def baddapp(*args):
+    ...     raise Exception("I'm a bad-app")
+
+    >>> worker = zc.resumelb.worker.Worker(baddapp, ('127.0.0.1', 0))
+    >>> worker_socket = gevent.socket.create_connection(worker.addr)
+    >>> zc.resumelb.util.read_message(worker_socket)
+    (0, {})
+
+    >>> env = newenv('', '/hi.html', dict(REQUEST_METHOD="HEAD"))
+    >>> handler = zope.testing.loggingsupport.InstalledHandler(
+    ...     'zc.resumelb.worker')
+    >>> zc.resumelb.util.write_message(worker_socket, 1, env, '')
+    >>> print_response(worker_socket, 1)
+    1 500 Internal Server Error
+    Content-Length: 23
+    Content-Type: text/html; charset=UTF-8
+    <BLANKLINE>
+
+    >>> for record in handler.records:
+    ...     print record.name, record.levelname
+    ...     print record.getMessage()
+    ...     if record.exc_info:
+    ...         traceback.print_exception(*record.exc_info)
+    ... # doctest: +ELLIPSIS
+    zc.resumelb.worker ERROR
+    Uncaught application exception for 1
+    Traceback (most recent call last):
+    ...
+    Exception: I'm a bad-app
+
+    >>> handler.uninstall()
+    >>> worker.stop()
+    """ #"
+
 def Buffering_Temporary_Files_are_closed():
     """
     When a worker sends data to an lb faster than it can send it to a

diff --git a/src/zc/resumelb/worker.py b/src/zc/resumelb/worker.py
@@ -84,7 +84,7 @@ def start_response(status, headers, exc_info=None):
                         'A system error occurred', status=500
                         )(env, start_response)
                     )
-                first = body.next()
+                first = ''
 
             return response[0], first, body
 

diff --git a/src/zc/resumelb/zk.py b/src/zc/resumelb/zk.py
@@ -191,7 +191,8 @@ def lbmain(args=None, run=True):
                 ZConfig.configureLoggers(f.read())
 
     zk = zc.zk.ZooKeeper(zookeeper)
-    addrs = zk.children(path+'/workers/providers')
+    worker_path = zk.resolve(path+'/workers/providers')
+    addrs = zk.children(worker_path)
     request_classifier = _resolve(options.request_classifier)
     pool_factory = _resolve(options.pool_factory)
 
@@ -222,7 +223,7 @@ def get_addrs(a):
             to_send[0] = dict(
                 (zc.parse_addr.parse_addr(addr),
                  zk.get_properties(
-                     path+'/workers/providers/'+addr).get('version')
+                     worker_path + '/' + addr).get('version')
                  )
                 for addr in addrs)
             awatcher.send()

diff --git a/src/zc/resumelb/zk.test b/src/zc/resumelb/zk.test
@@ -11,10 +11,11 @@ To demonstrate the integration, we'll set up a tree:
     ... /test
     ...   /lb
     ...     /providers
-    ...     /workers
-    ...       history = 32
-    ...       max_skill_age = 90
-    ...       /providers
+    ...     workers -> ../app
+    ...   /app
+    ...     history = 32
+    ...     max_skill_age = 90
+    ...     /providers
     ... """)
 
 There are some things to note about this tree:
@@ -23,6 +24,9 @@ There are some things to note about this tree:
   subnode where we'll configure the workers.  This subnode could be a
   regular node, or a symbolic link.
 
+  Here we made it a link because a) that's often useful and b) it was
+  broken in some cases. :)
+
 - Addresses of running workers and load balancers will be registered
   at provider subnodes.
 
@@ -68,7 +72,7 @@ Let's create a worker, making sure that ZConfig.configureLoggers was called.
     ...   with mock.patch('logging.basicConfig') as basicConfig:
     ...     worker = zc.resumelb.zk.worker(
     ...         app, None,
-    ...         zookeeper='zookeeper.example.com:2181', path='/test/lb/workers',
+    ...         zookeeper='zookeeper.example.com:2181', path='/test/app',
     ...         address='127.0.0.1:0', run=False, loggers='loggers',
     ...         resume_file='resume.mar',
     ...         description='test',
@@ -101,19 +105,19 @@ It loaded its resume from resume.mar:
 It registers its address as an ephemeral subnode of the provider's
 subnode of the given path:
 
-    >>> [waddr] = zk.get_children('/test/lb/workers/providers')
+    >>> [waddr] = zk.get_children('/test/app/providers')
     >>> waddr == "%s:%s" % worker.addr
     True
 
-    >>> meta = zk.get('/test/lb/workers/providers/' + waddr)[1]
+    >>> meta = zk.get('/test/app/providers/' + waddr)[1]
     >>> ephemeralOwner = (meta['ephemeralOwner'] if isinstance(meta, dict)
     ...                   else meta.ephemeralOwner)
     >>> bool(ephemeralOwner)
     True
 
 And provides some meta data:
 
-    >>> pprint(zk.get_properties('/test/lb/workers/providers/' + waddr))
+    >>> pprint(zk.get_properties('/test/app/providers/' + waddr))
     {u'description': u'test', u'pid': 4195, u'version': u'42'}
 
 Shutdown
@@ -128,7 +132,7 @@ worker down:
 
     >>> gevent.signal.call_args[0][1]()
     >>> gevent.sleep(.01)
-    >>> zk.get_children('/test/lb/workers/providers')
+    >>> zk.get_children('/test/app/providers')
     []
 
     >>> gevent.socket.create_connection(worker.addr) # doctest: +ELLIPSIS
@@ -146,7 +150,7 @@ Let's try again, but this time, don't set up logging:
     ...   with mock.patch('logging.basicConfig') as basicConfig:
     ...     worker = zc.resumelb.zk.worker(
     ...         app, None,
-    ...         zookeeper='zookeeper.example.com:2181', path='/test/lb/workers',
+    ...         zookeeper='zookeeper.example.com:2181', path='/test/app',
     ...         address='127.0.0.1:0', run=False,
     ...         resume_file='resume.mar',
     ...         )
@@ -160,7 +164,7 @@ Let's try again, but this time, set up basic logging:
     ...   with mock.patch('logging.basicConfig') as basicConfig:
     ...     worker = zc.resumelb.zk.worker(
     ...         app, None,
-    ...         zookeeper='zookeeper.example.com:2181', path='/test/lb/workers',
+    ...         zookeeper='zookeeper.example.com:2181', path='/test/app',
     ...         address='127.0.0.1:0', run=False, loggers='INFO',
     ...         resume_file='resume.mar',
     ...         )
@@ -174,7 +178,7 @@ Let's try again, but this time, set up basic logging with a numeric level:
     ...   with mock.patch('logging.basicConfig') as basicConfig:
     ...     worker = zc.resumelb.zk.worker(
     ...         app, None,
-    ...         zookeeper='zookeeper.example.com:2181', path='/test/lb/workers',
+    ...         zookeeper='zookeeper.example.com:2181', path='/test/app',
     ...         address='127.0.0.1:0', run=False, loggers='42',
     ...         resume_file='resume.mar',
     ...         )
@@ -280,7 +284,7 @@ We want to make sure that non-test mode works:
 
     >>> worker2_greenlet = gevent.spawn(zc.resumelb.zk.worker,
     ...     app, None,
-    ...     zookeeper='zookeeper.example.com:2181', path='/test/lb/workers',
+    ...     zookeeper='zookeeper.example.com:2181', path='/test/app',
     ...     address='127.0.0.1:0')
     >>> gevent.sleep(.01)
 
@@ -475,7 +479,7 @@ Finally, let's test that:
 
     >>> worker_greenlet = gevent.spawn(zc.resumelb.zk.worker,
     ...     app, None,
-    ...     zookeeper='zookeeper.example.com:2181', path='/test/lb/workers',
+    ...     zookeeper='zookeeper.example.com:2181', path='/test/app',
     ...     address='127.0.0.1:0')
 
     >>> with mock.patch('ZConfig.configureLoggers') as configureLoggers:
@@ -526,7 +530,7 @@ workers must register with a version, or they'll be ignored.
     ...   with mock.patch('logging.basicConfig') as basicConfig:
     ...     worker = zc.resumelb.zk.worker(
     ...         app, None,
-    ...         zookeeper='zookeeper.example.com:2181', path='/test/lb/workers',
+    ...         zookeeper='zookeeper.example.com:2181', path='/test/app',
     ...         address='127.0.0.1:0', run=False, version='42')
     >>> gevent.sleep(.01)
     >>> lb, server = zc.resumelb.zk.lbmain(
@@ -536,7 +540,7 @@ workers must register with a version, or they'll be ignored.
     ...   with mock.patch('logging.basicConfig') as basicConfig:
     ...     worker = zc.resumelb.zk.worker(
     ...         app, None,
-    ...         zookeeper='zookeeper.example.com:2181', path='/test/lb/workers',
+    ...         zookeeper='zookeeper.example.com:2181', path='/test/app',
     ...         address='127.0.0.1:0', run=False, version='42')
     >>> gevent.sleep(.01)
     >>> lb.pool
@@ -560,7 +564,7 @@ takes keyword arguments.
     ...   with mock.patch('logging.basicConfig') as basicConfig:
     ...     worker = zc.resumelb.zk.worker(
     ...         app, None,
-    ...         zookeeper='zookeeper.example.com:2181', path='/test/lb/workers',
+    ...         zookeeper='zookeeper.example.com:2181', path='/test/app',
     ...         address='127.0.0.1:0', run=False, version='42')
     >>> gevent.sleep(.01)