Change in vdsm[master]: vdscli: add a timeout option for plaintext connections

Yeela Kaplan has uploaded a new change for review.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

vdscli: add a timeout option for plaintext connections

Change-Id: I38ebcb5bfb8fe93a04b8693cfcc17a6c0f6785ce
Signed-off-by: Yeela Kaplan 
---
M lib/vdsm/vdscli.py
1 file changed, 10 insertions(+), 6 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/47/38947/1

diff --git a/lib/vdsm/vdscli.py b/lib/vdsm/vdscli.py
index 8339766..d40b8c0 100644
--- a/lib/vdsm/vdscli.py
+++ b/lib/vdsm/vdscli.py
@@ -53,10 +53,17 @@
     connection. This class reverts the change to avoid the concurrency
     issues.'''

+    def __init__(self, timeout=sslutils.SOCKET_DEFAULT_TIMEOUT):
+        xmlrpclib.Transport.__init__(self)
+        self.timeout = timeout
+
     def make_connection(self, host):
         '''Creates a new HTTPConnection to the host.'''
+
         self._connection = None
-        return xmlrpclib.Transport.make_connection(self, host)
+        httpCon = xmlrpclib.Transport.make_connection(self, host)
+        httpCon._conn.timeout = self.timeout
+        return httpCon

def __guessDefaults():
@@ -116,11 +123,8 @@
         server = xmlrpclib.ServerProxy('https://%s' % hostPort,
                                        wrap_transport(transport))
     else:
-        # TODO: add timeout option for HTTP connection
-        if timeout is not sslutils.SOCKET_DEFAULT_TIMEOUT:
-            raise NotImplementedError('timeout is not supported in'
-                                      ' plaintext connection')
-        transport = wrap_transport(SingleRequestTransport())
+        transport = wrap_transport(
+            SingleRequestTransport(timeout=timeout))
         server = xmlrpclib.Server('http://%s' % hostPort, transport)
     return server

[e-mail] has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 1:

Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16900/

Yeela Kaplan has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 1: Verified+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 1:

Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17072/

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16900/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17072/ : SUCCESS

[e-mail] has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 2:

Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16986/

[e-mail] has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 3:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 3:

Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16988/

Dima Kuznetsov has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 3: Code-Review+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 2:

Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17160/

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16986/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17160/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 3:

Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17162/

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 3:

Build Successful

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16988/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17162/ : SUCCESS

Yaniv Bronhaim has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 3: Code-Review+1

Yeela Kaplan has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 3: Verified+1

Francesco Romani has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 3: Code-Review+1

(2 comments)

looks good enough

https://gerrit.ovirt.org/#/c/38947/3/lib/vdsm/vdscli.py
File lib/vdsm/vdscli.py:

Line 55: 
Line 56:     def __init__(self, *args, **kwargs):
Line 57:         if 'timeout' in kwargs:
Line 58:             self.timeout = kwargs['timeout']
Line 59:             del kwargs['timeout']
maybe
  kwargs.pop('timeout')

?
Anyway, not very important.
Line 60:         else:
Line 61:             self.timeout = sslutils.SOCKET_DEFAULT_TIMEOUT
Line 62: 
Line 63:         xmlrpclib.Transport.__init__(self, *args, **kwargs)

Line 59:             del kwargs['timeout']
Line 60:         else:
Line 61:             self.timeout = sslutils.SOCKET_DEFAULT_TIMEOUT
Line 62: 
Line 63:         xmlrpclib.Transport.__init__(self, *args, **kwargs)
super()?

again, not very important.
Line 64: 
Line 65:     def make_connection(self, host):
Line 66:         '''Creates a new HTTPConnection to the host.'''
Line 67:

[e-mail] has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 4:

* Update tracker::#1225455::OK
* Check Bug-Url::OK
* Check Public Bug::#1225455::OK, public bug
* Check Product::#1225455::OK, Correct product oVirt
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[e-mail] has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 5:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Yeela Kaplan has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 5: Verified+1

[e-mail] has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 6:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Yaniv Bronhaim has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 6: Code-Review+1

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 6: Code-Review+2

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

vdscli: add a timeout option for plaintext connections

Change-Id: I38ebcb5bfb8fe93a04b8693cfcc17a6c0f6785ce
Signed-off-by: Yeela Kaplan 
Reviewed-on: https://gerrit.ovirt.org/38947
Continuous-Integration: Jenkins CI
Reviewed-by: Yaniv Bronhaim 
Reviewed-by: Dan Kenigsberg 
---
M lib/vdsm/vdscli.py
1 file changed, 15 insertions(+), 6 deletions(-)

Approvals:
  Yeela Kaplan: Verified
  Yaniv Bronhaim: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved

[e-mail] has posted comments on this change.

Change subject: vdscli: add a timeout option for plaintext connections
......................................................................

Patch Set 7:

* Update tracker::IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.