Nir Soffer has posted comments on this change. Change subject: WIP: POC: virt: common handling of exception ......................................................................
Patch Set 5: Code-Review+1 (4 comments) Nice, minor comment about FINISH/ERROR log https://gerrit.ovirt.org/#/c/54664/5/lib/vdsm/virt/api.py File lib/vdsm/virt/api.py: Line 27: Line 28: Line 29: _log = logging.getLogger("virt.api") Line 30: Line 31: With little work, we can convert it to vdsm.api module and wrap all the apis using the same code, getting consistent logs. Line 32: def method(func): Line 33: @wraps(func) Line 34: def wrapper(*args, **kwargs): Line 35: Line 30: Line 31: Line 32: def method(func): Line 33: @wraps(func) Line 34: def wrapper(*args, **kwargs): For self, you can do: wrapper(self, *args, **kwargs) It will not work on functions, but this is not generic decorator for everything but specific one for virt api, which use always methods. Line 35: Line 36: # TODO: maybe add a (cheap) cookie so we can unambiguosly correlate Line 37: # this START and FINISH log later, so it is easy to compute the API Line 38: # RTT and other metrics Line 34: def wrapper(*args, **kwargs): Line 35: Line 36: # TODO: maybe add a (cheap) cookie so we can unambiguosly correlate Line 37: # this START and FINISH log later, so it is easy to compute the API Line 38: # RTT and other metrics Using the logger name + thread name + function name with START/FINISH should be good enough. Line 39: _log.debug("START %s args=%s kwargs=%s", func.__name__, args, kwargs) Line 40: try: Line 41: # TODO: 'self' argument? Line 42: ret = func(*args, **kwargs) Line 40: try: Line 41: # TODO: 'self' argument? Line 42: ret = func(*args, **kwargs) Line 43: except Exception as e: Line 44: _log.exception("ERROR %s error=%s", func.__name__, e) We already have ERROR in the log level - maybe use FINISH - this way we can detect the end of the call using FINISH, and the result using response/error. In current storage code, the response is alway the same: "Run and protect: funcname, Response:", and you can detect the error (in these logs) using the response body. Line 45: if not isinstance(e, exception.VdsmException): Line 46: e = exception.GeneralException(str(e)) Line 47: return e.response() Line 48: -- To view, visit https://gerrit.ovirt.org/54664 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic186dc8fa062d8b3789c6057bba68fbbc23f311b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Amit Aviram <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
