From e197dd54544da848ec86c8843b883221c69afb31 Mon Sep 17 00:00:00 2001
From: Khai Do <zaro0508@gmail.com>
Date: Thu, 4 Dec 2014 00:16:09 -0800
Subject: [PATCH] Add a default http timeout for connections to jenkins

Without a timeout a script can hang forever when attempting
to connect to jenkins. This change sets a default timeout
of 2 minutes.  Selection of default value is pretty arbitrary
on this change.

Closes-Bug: #1273329
Change-Id: If84778231b88d78a02a89a56f38f95d6deada80a
---
 jenkins/__init__.py   | 14 ++++++++++----
 tests/test_jenkins.py | 24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/jenkins/__init__.py b/jenkins/__init__.py
index 21be5d7..b6c15e8 100644
--- a/jenkins/__init__.py
+++ b/jenkins/__init__.py
@@ -51,6 +51,7 @@ import json
 import six
 from six.moves.http_client import BadStatusLine
 from six.moves.urllib.error import HTTPError
+from six.moves.urllib.error import URLError
 from six.moves.urllib.parse import quote, urlencode
 from six.moves.urllib.request import Request, urlopen
 
@@ -59,6 +60,7 @@ LAUNCHER_COMMAND = 'hudson.slaves.CommandLauncher'
 LAUNCHER_JNLP = 'hudson.slaves.JNLPLauncher'
 LAUNCHER_WINDOWS_SERVICE = 'hudson.os.windows.ManagedWindowsServiceLauncher'
 
+DEFAULT_CONN_TIMEOUT = 120
 INFO = 'api/json'
 PLUGIN_INFO = 'pluginManager/api/json?depth=%(depth)s'
 CRUMB_URL = 'crumbIssuer/api/json'
@@ -146,7 +148,7 @@ def auth_headers(username, password):
 
 class Jenkins(object):
 
-    def __init__(self, url, username=None, password=None):
+    def __init__(self, url, username=None, password=None, timeout=DEFAULT_CONN_TIMEOUT):
         '''Create handle to Jenkins instance.
 
         All methods will raise :class:`JenkinsException` on failure.
@@ -154,6 +156,7 @@ class Jenkins(object):
         :param username: Server username, ``str``
         :param password: Server password, ``str``
         :param url: URL of Jenkins server, ``str``
+        :param timeout: Server connection timeout (in seconds), ``int``
         '''
         if url[-1] == '/':
             self.server = url
@@ -164,6 +167,7 @@ class Jenkins(object):
         else:
             self.auth = None
         self.crumb = None
+        self.timeout = timeout
 
     def _get_encoded_params(self, params):
         for k, v in params.items():
@@ -244,7 +248,8 @@ class Jenkins(object):
                 req.add_header('Authorization', self.auth)
             if add_crumb:
                 self.maybe_add_crumb(req)
-            return urlopen(req).read()
+            response = urlopen(req, timeout=self.timeout).read()
+            return response
         except HTTPError as e:
             # Jenkins's funky authentication means its nigh impossible to
             # distinguish errors.
@@ -260,7 +265,8 @@ class Jenkins(object):
                 )
             elif e.code == 404:
                 raise NotFoundException('Requested item could not be found')
-            # right now I'm getting 302 infinites on a successful delete
+        except URLError as e:
+                raise JenkinsException('Error in request: %s' % (e.reason))
 
     def get_build_info(self, name, number, depth=0):
         '''Get build information dictionary.
@@ -372,7 +378,7 @@ class Jenkins(object):
         try:
             request = Request(self.server)
             request.add_header('X-Jenkins', '0.0')
-            response = urlopen(request)
+            response = urlopen(request, timeout=self.timeout)
             return response.info().getheader('X-Jenkins')
         except HTTPError:
             raise JenkinsException("Error communicating with server[%s]"
diff --git a/tests/test_jenkins.py b/tests/test_jenkins.py
index 030111f..c8d3631 100644
--- a/tests/test_jenkins.py
+++ b/tests/test_jenkins.py
@@ -77,6 +77,14 @@ class JenkinsTest(unittest.TestCase):
         self.assertEqual(j.auth.decode(), 'Basic %s' % (
             long_str_b64 + 'Om' + long_str_b64[2:] + 'YQ=='))
 
+    def test_constructor_default_timeout(self):
+        j = jenkins.Jenkins('http://example.com')
+        self.assertEqual(j.timeout, 120)
+
+    def test_constructor_custom_timeout(self):
+        j = jenkins.Jenkins('http://example.com', timeout=300)
+        self.assertEqual(j.timeout, 300)
+
     @patch.object(jenkins.Jenkins, 'jenkins_open')
     def test_get_job_config_encodes_job_name(self, jenkins_mock):
         j = jenkins.Jenkins('http://example.com/', 'test', 'test')
@@ -182,6 +190,22 @@ class JenkinsTest(unittest.TestCase):
             jenkins_mock.call_args[0][0].get_full_url(),
             'http://example.com/job/TestJob')
 
+    @patch('jenkins.urlopen')
+    def test_jenkins_open__timeout(self, jenkins_mock):
+        jenkins_mock.side_effect = jenkins.URLError(
+            reason="timed out")
+        j = jenkins.Jenkins('http://example.com/', 'test', 'test', timeout=1)
+        request = jenkins.Request('http://example.com/job/TestJob')
+
+        with self.assertRaises(jenkins.JenkinsException) as context_manager:
+            j.jenkins_open(request, add_crumb=False)
+        self.assertEqual(
+            str(context_manager.exception),
+            'Error in request: timed out')
+        self.assertEqual(
+            jenkins_mock.call_args[0][0].get_full_url(),
+            'http://example.com/job/TestJob')
+
     @patch.object(jenkins.Jenkins, 'jenkins_open')
     def test_assert_job_exists__job_missing(self, jenkins_mock):
         jenkins_mock.side_effect = jenkins.NotFoundException()