Change in vdsm[master]: xmlrpc: fd leak fix

nsoffer at redhat.com nsoffer at redhat.com
Wed Apr 15 16:41:34 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: xmlrpc: fd leak fix
......................................................................


Patch Set 6:

(4 comments)

https://gerrit.ovirt.org/#/c/39506/6/lib/yajsonrpc/betterAsyncore.py
File lib/yajsonrpc/betterAsyncore.py:

Line 186:         self._map = {}
Line 187:         self._is_running = False
Line 188:         self._wakeupEvent = AsyncoreEvent(self._map)
Line 189: 
Line 190:     def create_dispatcher(self, client, impl=None):
Please use the same terms used by the dispatcher - "client" should be "sock"
Line 191:         return Dispatcher(
Line 192:             impl=impl,
Line 193:             sock=client,
Line 194:             map=self._map


https://gerrit.ovirt.org/#/c/39506/6/vdsm/protocoldetector.py
File vdsm/protocoldetector.py:

Line 167
Line 168
Line 169
Line 170
Line 171
This indentation cause my eyes to bleed. Lets use more readable indentation:

    self._acceptor = self._reactor.create_dispatcher(
        sock, _AcceptorImpl(self._handle_accept))


Line 174:         if self._sslctx is None:
Line 175:             self._register_protocol_detector(
Line 176:                 self._reactor.create_dispatcher(
Line 177:                     client
Line 178:                 )
Are you sure this works? the first parameter is the socket?

Can we format this in a more readable way?

    def handle_accept(self, client):
        if self._sslctx is None:
            dispatcher = self._reactor.create_dispatcher(client)
            self._register_protocol_detector(dispatcher)
Line 179:             )
Line 180:         else:
Line 181:             self._reactor.create_dispatcher(
Line 182:                 client,


Line 183:                 SSLHandshakeDispatcher(
Line 184:                     self._sslctx,
Line 185:                     self._register_protocol_detector,
Line 186:                     self.TIMEOUT,
Line 187:                 )
Same indentation can be used here.
Line 188:             )
Line 189: 
Line 190:     def _register_protocol_detector(self, dispatcher):
Line 191:         dispatcher.switch_implementation(


-- 
To view, visit https://gerrit.ovirt.org/39506
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I705b0cf39937fe8d175305ad8ea8ad615f3ffb49
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list