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 <stira...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Lev Veyde <lve...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Yedidyah Bar David <d...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to