On Tue, Jan 15, 2013 at 03:40:17PM +0200, Dan Kenigsberg wrote:
On Mon, Jan 07, 2013 at 03:10:38PM -0500, danken@redhat.com wrote:
Dan Kenigsberg has posted comments on this change.
Change subject: Refactor communication infra ......................................................................
Patch Set 15: (2 inline comments)
.................................................... File vdsm_api/BindingJsonRpc.py Line 37: self._reactors = reactors Line 38: Line 39: def _createTcpReactor(self, cfg): Line 40: address = cfg.get("ip", "0.0.0.0") Line 41: port = cfg.get("port", 4044) sorry, I do not follow. which error handling code would have been required? to handle the case of not having cfg['ip'] ? having a default here only hide such an error instead of crying out loud about it.
Saggi, Adam, could you help me here?
I do not understand the need for the default port here. Could you elaborate on the justification?
I'll give Saggi a chance to defend it. On the face of it I'd agree with you Dan that the initialization code in clientIF should be structured similarly to the BindingXMLRPC code. Configuration for all ports (regardless of how the underlying code is structured) should probably be in a flat namespace and together in the vdsm.conf file.