Use explicit WBE object arguments (instead of kwargs)

Instead of passing around kwargs to root WBE classes and to
contained classes prefer to use explicitnamed arguments that are
passed around. This makes the code more obvious as to what
the intended arguments are and makes it easier for error validation
when other unknown arguments are passed (as well as for docs).

Also moves the docs for the worker engine to be a sub-TOC under
the main engine document so that it can be more easily explored
and managed/found...

Change-Id: I9413fad187c330fee494f0d4536cc27d9a90f0fb

doc/source/engines.rst

@@ -187,8 +187,16 @@ Worker-based
 
 **Engine type**: ``'worker-based'``
 
-For more information, please see :doc:`workers <workers>` for more details on
+.. note:: Since this engine is significantly more complicated (and
-how the worker based engine operates (and the design decisions behind it).
+          different) then the others we thought it appropriate to devote a
+          whole documentation section to it.
+
+For further information, please refer to the the following:
+
+.. toctree::
+   :maxdepth: 2
+
+   workers
 
 How they run
 ============

doc/source/index.rst

@@ -29,11 +29,6 @@ Contents
    jobs
    conductors
 
-.. toctree::
-   :hidden:
-
-   workers
-
 Examples
 --------
 

doc/source/workers.rst

@@ -1,7 +1,3 @@
--------
-Workers
--------
-
 Overview
 ========
 
@@ -98,9 +94,9 @@ Use-cases
 Design
 ======
 
-There are two communication sides, the *executor* and *worker* that communicate
+There are two communication sides, the *executor* (and associated engine
-using a proxy component. The proxy is designed to accept/publish messages
+derivative) and *worker* that communicate using a proxy component. The proxy
-from/into a named exchange.
+is designed to accept/publish messages from/into a named exchange.
 
 High level architecture
 -----------------------
@@ -390,7 +386,7 @@ Limitations
 Interfaces
 ==========
 
-.. automodule:: taskflow.engines.worker_based.worker
 .. automodule:: taskflow.engines.worker_based.engine
-.. automodule:: taskflow.engines.worker_based.proxy
 .. automodule:: taskflow.engines.worker_based.executor
+.. automodule:: taskflow.engines.worker_based.proxy
+.. automodule:: taskflow.engines.worker_based.worker

taskflow/engines/worker_based/executor.py

@@ -67,14 +67,16 @@ class WorkerTaskExecutor(executor.TaskExecutor):
     """Executes tasks on remote workers."""
 
     def __init__(self, uuid, exchange, topics,
-                 transition_timeout=pr.REQUEST_TIMEOUT, **kwargs):
+                 transition_timeout=pr.REQUEST_TIMEOUT,
+                 url=None, transport=None, transport_options=None,
+                 retry_options=None):
         self._uuid = uuid
         self._topics = topics
         self._requests_cache = cache.RequestsCache()
         self._transition_timeout = transition_timeout
         self._workers_cache = cache.WorkersCache()
         self._workers_arrival = threading.Condition()
-        handlers = {
+        type_handlers = {
             pr.NOTIFY: [
                 self._process_notify,
                 functools.partial(pr.Notify.validate, response=True),
@@ -84,8 +86,11 @@ class WorkerTaskExecutor(executor.TaskExecutor):
                 pr.Response.validate,
             ],
         }
-        self._proxy = proxy.Proxy(uuid, exchange, handlers,
+        self._proxy = proxy.Proxy(uuid, exchange, type_handlers,
-                                  self._on_wait, **kwargs)
+                                  on_wait=self._on_wait, url=url,
+                                  transport=transport,
+                                  transport_options=transport_options,
+                                  retry_options=retry_options)
         self._proxy_thread = None
         self._periodic = PeriodicWorker(tt.Timeout(pr.NOTIFY_PERIOD),
                                         [self._notify_topics])

taskflow/engines/worker_based/proxy.py

@@ -64,10 +64,12 @@ class Proxy(object):
     # value is valid...
     _RETRY_INT_OPTS = frozenset(['max_retries'])
 
-    def __init__(self, topic, exchange_name, type_handlers, on_wait=None,
+    def __init__(self, topic, exchange, type_handlers,
-                 **kwargs):
+                 on_wait=None, url=None,
+                 transport=None, transport_options=None,
+                 retry_options=None):
         self._topic = topic
-        self._exchange_name = exchange_name
+        self._exchange_name = exchange
         self._on_wait = on_wait
         self._running = threading_utils.Event()
         self._dispatcher = dispatcher.TypeDispatcher(type_handlers)
@@ -76,18 +78,13 @@ class Proxy(object):
             # running, otherwise requeue them.
             lambda data, message: not self.is_running)
 
-        # TODO(harlowja): make these keyword arguments explict...
-        url = kwargs.get('url')
-        transport = kwargs.get('transport')
-        transport_opts = kwargs.get('transport_options')
         ensure_options = self._DEFAULT_RETRY_OPTIONS.copy()
-        if 'retry_options' in kwargs and kwargs['retry_options'] is not None:
+        if retry_options is not None:
             # Override the defaults with any user provided values...
-            usr_retry_options = kwargs['retry_options']
             for k in set(six.iterkeys(ensure_options)):
-                if k in usr_retry_options:
+                if k in retry_options:
                     # Ensure that the right type is passed in...
-                    val = usr_retry_options[k]
+                    val = retry_options[k]
                     if k in self._RETRY_INT_OPTS:
                         tmp_val = int(val)
                     else:
@@ -100,14 +97,14 @@ class Proxy(object):
         self._ensure_options = ensure_options
 
         self._drain_events_timeout = DRAIN_EVENTS_PERIOD
-        if transport == 'memory' and transport_opts:
+        if transport == 'memory' and transport_options:
-            polling_interval = transport_opts.get('polling_interval')
+            polling_interval = transport_options.get('polling_interval')
             if polling_interval is not None:
                 self._drain_events_timeout = polling_interval
 
         # create connection
         self._conn = kombu.Connection(url, transport=transport,
-                                      transport_options=transport_opts)
+                                      transport_options=transport_options)
 
         # create exchange
         self._exchange = kombu.Exchange(name=self._exchange_name,

taskflow/engines/worker_based/server.py

@@ -45,8 +45,10 @@ def delayed(executor):
 class Server(object):
     """Server implementation that waits for incoming tasks requests."""
 
-    def __init__(self, topic, exchange, executor, endpoints, **kwargs):
+    def __init__(self, topic, exchange, executor, endpoints,
-        handlers = {
+                 url=None, transport=None, transport_options=None,
+                 retry_options=None):
+        type_handlers = {
             pr.NOTIFY: [
                 delayed(executor)(self._process_notify),
                 functools.partial(pr.Notify.validate, response=False),
@@ -56,8 +58,10 @@ class Server(object):
                 pr.Request.validate,
             ],
         }
-        self._proxy = proxy.Proxy(topic, exchange, handlers,
+        self._proxy = proxy.Proxy(topic, exchange, type_handlers,
-                                  on_wait=None, **kwargs)
+                                  url=url, transport=transport,
+                                  transport_options=transport_options,
+                                  retry_options=retry_options)
         self._topic = topic
         self._endpoints = dict([(endpoint.name, endpoint)
                                 for endpoint in endpoints])

taskflow/tests/unit/worker_based/test_executor.py

@@ -84,10 +84,13 @@ class TestWorkerTaskExecutor(test.MockTestCase):
 
     def test_creation(self):
         ex = self.executor(reset_master_mock=False)
-
         master_mock_calls = [
             mock.call.Proxy(self.executor_uuid, self.executor_exchange,
-                            mock.ANY, ex._on_wait, url=self.broker_url)
+                            mock.ANY, on_wait=ex._on_wait,
+                            url=self.broker_url, transport=mock.ANY,
+                            transport_options=mock.ANY,
+                            retry_options=mock.ANY
+                            )
         ]
         self.assertEqual(self.master_mock.mock_calls, master_mock_calls)
 
@@ -250,8 +253,7 @@ class TestWorkerTaskExecutor(test.MockTestCase):
         self.assertEqual(expected_calls, self.master_mock.mock_calls)
 
     def test_execute_task_topic_not_found(self):
-        workers_info = {self.executor_topic: ['<unknown>']}
+        ex = self.executor()
-        ex = self.executor(workers_info=workers_info)
         ex.execute_task(self.task, self.task_uuid, self.task_args)
 
         expected_calls = [

taskflow/tests/unit/worker_based/test_proxy.py

@@ -28,7 +28,7 @@ class TestProxy(test.MockTestCase):
         super(TestProxy, self).setUp()
         self.topic = 'test-topic'
         self.broker_url = 'test-url'
-        self.exchange_name = 'test-exchange'
+        self.exchange = 'test-exchange'
         self.timeout = 5
         self.de_period = proxy.DRAIN_EVENTS_PERIOD
 
@@ -72,7 +72,7 @@ class TestProxy(test.MockTestCase):
         self.resetMasterMock()
 
     def _queue_name(self, topic):
-        return "%s_%s" % (self.exchange_name, topic)
+        return "%s_%s" % (self.exchange, topic)
 
     def proxy_start_calls(self, calls, exc_type=mock.ANY):
         return [
@@ -119,7 +119,7 @@ class TestProxy(test.MockTestCase):
 
     def proxy(self, reset_master_mock=False, **kwargs):
         proxy_kwargs = dict(topic=self.topic,
-                            exchange_name=self.exchange_name,
+                            exchange=self.exchange,
                             url=self.broker_url,
                             type_handlers={})
         proxy_kwargs.update(kwargs)
@@ -134,7 +134,7 @@ class TestProxy(test.MockTestCase):
         master_mock_calls = [
             mock.call.Connection(self.broker_url, transport=None,
                                  transport_options=None),
-            mock.call.Exchange(name=self.exchange_name,
+            mock.call.Exchange(name=self.exchange,
                                durable=False,
                                auto_delete=True)
         ]
@@ -147,7 +147,7 @@ class TestProxy(test.MockTestCase):
         master_mock_calls = [
             mock.call.Connection(self.broker_url, transport='memory',
                                  transport_options=transport_opts),
-            mock.call.Exchange(name=self.exchange_name,
+            mock.call.Exchange(name=self.exchange,
                                durable=False,
                                auto_delete=True)
         ]

taskflow/tests/unit/worker_based/test_server.py

@@ -86,7 +86,9 @@ class TestServer(test.MockTestCase):
         # check calls
         master_mock_calls = [
             mock.call.Proxy(self.server_topic, self.server_exchange,
-                            mock.ANY, url=self.broker_url, on_wait=mock.ANY)
+                            mock.ANY, url=self.broker_url,
+                            transport=mock.ANY, transport_options=mock.ANY,
+                            retry_options=mock.ANY)
         ]
         self.master_mock.assert_has_calls(master_mock_calls)
         self.assertEqual(len(s._endpoints), 3)
@@ -97,7 +99,9 @@ class TestServer(test.MockTestCase):
         # check calls
         master_mock_calls = [
             mock.call.Proxy(self.server_topic, self.server_exchange,
-                            mock.ANY, url=self.broker_url, on_wait=mock.ANY)
+                            mock.ANY, url=self.broker_url,
+                            transport=mock.ANY, transport_options=mock.ANY,
+                            retry_options=mock.ANY)
         ]
         self.master_mock.assert_has_calls(master_mock_calls)
         self.assertEqual(len(s._endpoints), len(self.endpoints))

taskflow/tests/unit/worker_based/test_worker.py

@@ -82,7 +82,8 @@ class TestWorker(test.MockTestCase):
         master_mock_calls = [
             mock.call.executor_class(10),
             mock.call.Server(self.topic, self.exchange,
-                             self.executor_inst_mock, [], url=self.broker_url)
+                             self.executor_inst_mock, [],
+                             url=self.broker_url)
         ]
         self.assertEqual(self.master_mock.mock_calls, master_mock_calls)