Change in vdsm[master]: MonkeyPatch: fix reverting staticmethods.

danken at redhat.com danken at redhat.com
Fri Jan 17 14:59:01 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: MonkeyPatch: fix reverting staticmethods.
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

http://gerrit.ovirt.org/#/c/23378/1//COMMIT_MSG
Commit Message:

Line 4: Commit:     Antoni S. Puimedon <asegurap at redhat.com>
Line 5: CommitDate: 2014-01-17 12:38:52 +0100
Line 6: 
Line 7: MonkeyPatch: fix reverting staticmethods.
Line 8: 
Let's be more specific:

In Python 2, setattr(Class, name, func) automatically converts func into an instancemethod. To keep type(Class.func) as function , staticmethod(func) must be applied explicitly.
Line 9: Under some circumstances setattr can make an object of type function
Line 10: become of type instancemethod. This patch detects those cases and
Line 11: fixes the type. This is specially relevant when reverting a
Line 12: @staticmethod.


http://gerrit.ovirt.org/#/c/23378/1/tests/monkeypatch.py
File tests/monkeypatch.py:

Line 66:     def revert(self):
Line 67:         assert self.old != []
Line 68:         while self.old:
Line 69:             module, name, that = self.old.pop()
Line 70:             oldtype = type(that)
> nit: either oldType and isFunction or oldtype and isfunction, IMO. However,
we do not really need the valiable "isFunction", and we can tuck all fixup code in one block ("that" does not change its type while being set as an attribute to the class).
Line 71:             isFunction = isinstance(that, types.FunctionType)
Line 72:             setattr(module, name, that)
Line 73:             if oldtype != type(getattr(module, name)):
Line 74:                 if isFunction:


Line 69:             module, name, that = self.old.pop()
Line 70:             oldtype = type(that)
Line 71:             isFunction = isinstance(that, types.FunctionType)
Line 72:             setattr(module, name, that)
Line 73:             if oldtype != type(getattr(module, name)):
> Simpler usecase to highlight the issue:
+1 for adding a comment including a note that this odd code can be removed when Python 2 is no longer supported.
Line 74:                 if isFunction:
Line 75:                     setattr(module, name, staticmethod(that))
Line 76: 
Line 77: 


-- 
To view, visit http://gerrit.ovirt.org/23378
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3100455ab761481f57a93ab8aacc34ac4ab91eb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list