Piotr Kliczewski has posted comments on this change. Change subject: xmlrpc: fd leak fix ......................................................................
Patch Set 3: (3 comments) https://gerrit.ovirt.org/#/c/39506/3/lib/yajsonrpc/betterAsyncore.py File lib/yajsonrpc/betterAsyncore.py: Line 75: def _unbind_implementation(self): Line 76: """ Line 77: We need to unbind all the methods in order Line 78: to be able to garbage collect Dispatcher object. Line 79: """ > Ok with the content, but worded this way it is IMHO more apt for comment th Done Line 80: for attr_name in ( Line 81: "handle_accept", Line 82: "handle_close", Line 83: "handle_connect", Line 86: "handle_read", Line 87: "handle_write", Line 88: "readable", Line 89: "writable", Line 90: ): > minor: This tuple can be moved to a class-level constant to make sure that Done Line 91: setattr( Line 92: self, Line 93: attr_name, Line 94: lambda: None Line 91: setattr( Line 92: self, Line 93: attr_name, Line 94: lambda: None Line 95: ) > If we are 100% sure the object is going to be destroyed soon, could make se The object is still in progress in asyncore.loop and there are still methods call on it. I will use: lambda *args: None Line 96: Line 97: def next_check_interval(self): Line 98: """ Line 99: Return the relative timeout wanted between poller refresh checks -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
