Bug 54067 – nrwt multiprocessing: minor cleanup prior to implementing the new worker

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dirk Pranke]

nrwt multiprocessing: minor cleanup prior to implementing the new worker

[Dirk Pranke]

Created attachment 81753 [details]
Patch

[Dirk Pranke]

Created attachment 81754 [details]
make ManagerConnection inherit from BrokerConnection

[Tony Chang]

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

[Dirk Pranke]

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?

[Tony Chang]

(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.

[Dirk Pranke]

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.

[Dirk Pranke]

Comment on attachment 81754 [details]
make ManagerConnection inherit from BrokerConnection

Clearing flags on attachment: 81754

Committed r78159: <http://trac.webkit.org/changeset/78159>

[Dirk Pranke]

All reviewed patches have been landed.  Closing bug.