Francesco Romani has posted comments on this change. Change subject: WIP: POC: virt: common handling of exception ......................................................................
Patch Set 5: (4 comments) 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 api Nice idea, I'm just worried about name clash again: vdsm/API.py - I guess this will never be moved under lib/ ? lib/vdsm/api.py - ok, case is different, but looks too similar. 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: I will check that, I'm just not sure it will work transparently with plain functions and bound methods. Let me play with it and report here. 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 shoul OK, so will remove this TODO and go ahead. 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 good idea, let's always use the same identifier (FINISH) 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
