Nir Soffer has posted comments on this change.
Change subject: json-rpc: Protocol detection
......................................................................
Patch Set 32:
(1 comment)
http://gerrit.ovirt.org/#/c/26300/32/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:
Line 488: res = True if res is None else res
Line 489:
Line 490: ctx.requestDone(JsonRpcResponse(res, None, req.id))
Line 491:
Line 492: @traceback(on=log.name)
Nir suggested to use it.
I was suggesting this on the protocol
detector, not here, because I did not review this file.
This decorator makes code simpler because you don't need to add try blocks around the
body of threads target function, and it will not break when your error handling code is
broken.
This does not replace proper error handling in the thread, but a last resort error
handler, ensuring that when the thread dies in the field (because error handling was
broken), it will be logged.
So instead of:
def mainloop(self):
try:
do stuff
and other stuff
even more
except Exception:
logging.exception("thread foo died")
You can do:
@traceback(on=log.name, msg="thread foo died")
def mainloop(self):
do stuff
and other stuff
even more
Looking in function body, there is no error handling in the while loop, so any unexpected
failure in one of statements will cause the thread to abort silently, which makes it
impossible to support this code in the field.
Line 493: def serve_requests(self):
Line 494: while True:
Line 495: self.log.debug("Waiting for request")
Line 496: obj = self._workQueue.get()
--
To view, visit
http://gerrit.ovirt.org/26300
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75
Gerrit-PatchSet: 32
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Liron Ar <laravot(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes