Nir Soffer has posted comments on this change.

Change subject: lib: api.py: preserve signature in decorator
......................................................................


Patch Set 7:

(3 comments)

https://gerrit.ovirt.org/#/c/63759/7/lib/vdsm/common/api.py
File lib/vdsm/common/api.py:

Line 29: 
Line 30: _log = logging.getLogger("virt.api")
Line 31: 
Line 32: 
Line 33: @decorator
Nice!

Sad that we need such hacks for such basic code, but we probably need this on 
all decorators.

Did you inspect the decorator module code and tested its performance compared 
with old code?

For example, I would like to see timeit output before and after this patch, 
calling decorated method.
Line 34: def method(func, *args, **kwargs):
Line 35:     """
Line 36:     Decorate an instance method, and return a response according to the
Line 37:     outcome of the call.


https://gerrit.ovirt.org/#/c/63759/7/vdsm.spec.in
File vdsm.spec.in:

Line 109: %endif # support python3
Line 110: 
Line 111: %if 0%{?rhel}
Line 112: BuildRequires: python-decorator
Line 113: %else # fedora
Please have explicit branch for each platform, no more "else # fedora", this 
starts small, and quickly becomes impossible to maintain.
Line 114: BuildRequires: python2-decorator
Line 115: %endif # rhel
Line 116: 
Line 117: # Autotools BuildRequires


Line 144: Requires: iproute >= 3.10.0
Line 145: Requires: PyYAML
Line 146: %if 0%{?rhel}
Line 147: Requires: python-decorator
Line 148: %else # fedora
Same - best to have one block for platform specific requirements , not block 
per requirement, maybe at the end of the this list?
Line 149: Requires: python2-decorator
Line 150: %endif # rhel
Line 151: Requires: python-netaddr
Line 152: Requires: python-inotify


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06205cf180229ea19ffc38a7b88346afc18f13e4
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Irit Goihman <igoih...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Reply via email to