Bug 54067

Summary: nrwt multiprocessing: minor cleanup prior to implementing the new worker
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: New BugsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, mihaip, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 49566, 54068    
Attachments:
Description Flags
Patch
none
make ManagerConnection inherit from BrokerConnection none

Description Dirk Pranke 2011-02-08 22:32:54 PST
nrwt multiprocessing: minor cleanup prior to implementing the new worker
Comment 1 Dirk Pranke 2011-02-08 22:34:39 PST
Created attachment 81753 [details]
Patch
Comment 2 Dirk Pranke 2011-02-08 22:48:30 PST
Created attachment 81754 [details]
make ManagerConnection inherit from BrokerConnection
Comment 3 Tony Chang 2011-02-09 11:00:39 PST
Comment on attachment 81754 [details]
make ManagerConnection inherit from BrokerConnection

View in context: https://bugs.webkit.org/attachment.cgi?id=81754&action=review

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker.py:139
> +class _ManagerConnection(message_broker2.BrokerConnection):

It might be good to somehow make it clear in the diagram in message_broker2.py that BrokerConnection is always a Manager.   Something like:

50	    BrokerClient   ------>    ManagerConnection (implements BrokerConnection)
51	         ^                         |
52	         |                         v
53	         \----------------      Broker
Comment 4 Dirk Pranke 2011-02-09 12:18:53 PST
WorkerConnection (see bug 54067) will also implement BrokerConnection, just as the Worker class will implement AbstractWorker (and hence BrokerClient). Also, from the broker's point of view, there's no such thing as a "manager", only clients. It might make sense to create a similar drawing in manager_worker_broker showing both sides.

Thoughts?
Comment 5 Tony Chang 2011-02-09 12:28:18 PST
(In reply to comment #4)
> WorkerConnection (see bug 54067) will also implement BrokerConnection, just as the Worker class will implement AbstractWorker (and hence BrokerClient). Also, from the broker's point of view, there's no such thing as a "manager", only clients. It might make sense to create a similar drawing in manager_worker_broker showing both sides.
> 
> Thoughts?

Ah, right, I see.  Seems like in manager_worker_broker.py, we could have a diagram with the concrete classes on the manager side and in whatever file has the Worker implementations, it could have a diagram with the worker side.
Comment 6 Dirk Pranke 2011-02-09 16:07:03 PST
er, that was supposed to be bug 54068, not 54067 (which is this bug).

At any rate, I will add a diagram in that bug, since most of the classes don't exist in this patch.
Comment 7 Dirk Pranke 2011-02-09 16:09:41 PST
Comment on attachment 81754 [details]
make ManagerConnection inherit from BrokerConnection

Clearing flags on attachment: 81754

Committed r78159: <http://trac.webkit.org/changeset/78159>
Comment 8 Dirk Pranke 2011-02-09 16:09:45 PST
All reviewed patches have been landed.  Closing bug.