Change in vdsm[master]: packaging: debian: making code compatible with upstream M2Cr...

Alon Bar-Lev alonbl at redhat.com
Fri Feb 13 10:43:32 UTC 2015


Alon Bar-Lev has posted comments on this change.

Change subject: packaging: debian: making code compatible with upstream M2Crypto
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/37746/4/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:

Line 40:     def gettimeout(self):
Line 41:         return self.connection.socket.gettimeout()
Line 42: 
Line 43:     @staticmethod
Line 44:     def setSSLConnectionTimeout(conn, timeout):
> It's static because it's not really using any object data and I want to mak
I completely disagree.

you have self.connection in SSLSocket base, you can use it and remove the static, and provide the settimeout for the SSLSocket class that does whatever magic it requires. this should be a method not a utility, basic ood.

it is not important what fedora applies, if you want to support several environment, you need to use the common functionality.
Line 45:         msettimeout = getattr(conn, 'settimeout', None)
Line 46:         if msettimeout:
Line 47:             r = msettimeout(timeout)
Line 48:         else:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id1808bc9e27dde72b018c3be413f5b2066127982
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Simone Tiraboschi <stirabos at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Lev Veyde <lveyde at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo at redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stirabos at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yedidyah Bar David <didi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list