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