[ARVADOS] updated: 1fa1006e5b21cfc30e694890db969c20864af8c6

git at public.curoverse.com git at public.curoverse.com
Fri Nov 7 15:40:12 EST 2014


Summary of changes:
 sdk/python/arvados/keep.py           | 26 ++++++++++++++++++-----
 sdk/python/tests/test_keep_client.py | 40 ++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 5 deletions(-)

       via  1fa1006e5b21cfc30e694890db969c20864af8c6 (commit)
      from  3993ba462c113090bc2db79db30838c017a5aa3b (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 1fa1006e5b21cfc30e694890db969c20864af8c6
Author: Tim Pierce <twp at curoverse.com>
Date:   Fri Nov 7 15:38:40 2014 -0500

    3857: implement proxy_timeout
    
    Add proxy_timeout argument to KeepClient.
    
    Default timeouts are:
    * timeout = (3, 30)
    * proxy_timeout = (20, 60)
    
    Added unit tests to check that KeepClient.get and KeepClient.put use the
    default 'timeout' setting when talking directly to a Keep server, and
    the 'proxy_timeout' setting when communicating with a proxy.

diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py
index d307363..d927580 100644
--- a/sdk/python/arvados/keep.py
+++ b/sdk/python/arvados/keep.py
@@ -209,6 +209,14 @@ class KeepBlockCache(object):
             self._cache_lock.release()
 
 class KeepClient(object):
+
+    # Default Keep server connection timeout:  3 seconds
+    # Default Keep server read timeout:       30 seconds
+    # Default Keep proxy connection timeout:  20 seconds
+    # Default Keep proxy read timeout:        60 seconds
+    DEFAULT_TIMEOUT = (3, 30)
+    DEFAULT_PROXY_TIMEOUT = (20, 60)
+
     class ThreadLimiter(object):
         """
         Limit the number of threads running at a given time to
@@ -393,7 +401,8 @@ class KeepClient(object):
                               self.service.last_result.text)
 
 
-    def __init__(self, api_client=None, proxy=None, timeout=300,
+    def __init__(self, api_client=None, proxy=None,
+                 timeout=DEFAULT_TIMEOUT, proxy_timeout=DEFAULT_PROXY_TIMEOUT,
                  api_token=None, local_store=None, block_cache=None,
                  num_retries=0):
         """Initialize a new KeepClient.
@@ -406,9 +415,14 @@ class KeepClient(object):
           Keep proxy.  Otherwise, KeepClient will fall back to the setting
           of the ARVADOS_KEEP_PROXY configuration setting.  If you want to
           ensure KeepClient does not use a proxy, pass in an empty string.
-        * timeout: The timeout for all HTTP requests, in seconds.  Default
-          300. A tuple of two floats is interpreted as (connection_timeout,
-          read_timeout)
+        * timeout: The timeout (in seconds) for HTTP requests to Keep
+          non-proxy servers.  A tuple of two floats is interpreted as
+          (connection_timeout, read_timeout): see
+          http://docs.python-requests.org/en/latest/user/advanced/#timeouts.
+          Default: (3, 30).
+        * proxy_timeout: The timeout (in seconds) for HTTP requests to
+          Keep proxies. A tuple of two floats is interpreted as
+          (connection_timeout, read_timeout). Default: (20, 60).
         * api_token: If you're not using an API client, but only talking
           directly to a Keep proxy, this parameter specifies an API token
           to authenticate Keep requests.  It is an error to specify both
@@ -423,6 +437,7 @@ class KeepClient(object):
         * num_retries: The default number of times to retry failed requests.
           This will be used as the default num_retries value when get() and
           put() are called.  Default 0.
+
         """
         self.lock = threading.Lock()
         if proxy is None:
@@ -445,7 +460,6 @@ class KeepClient(object):
             self.get = self.local_store_get
             self.put = self.local_store_put
         else:
-            self.timeout = timeout
             self.num_retries = num_retries
             if proxy:
                 if not proxy.endswith('/'):
@@ -453,6 +467,7 @@ class KeepClient(object):
                 self.api_token = api_token
                 self.service_roots = [proxy]
                 self.using_proxy = True
+                self.timeout = proxy_timeout
                 self.static_service_roots = True
             else:
                 # It's important to avoid instantiating an API client
@@ -463,6 +478,7 @@ class KeepClient(object):
                 self.api_token = api_client.api_token
                 self.service_roots = None
                 self.using_proxy = None
+                self.timeout = timeout
                 self.static_service_roots = False
 
     def build_service_roots(self, force_rebuild=False):
diff --git a/sdk/python/tests/test_keep_client.py b/sdk/python/tests/test_keep_client.py
index 6a37a1b..feb0dd5 100644
--- a/sdk/python/tests/test_keep_client.py
+++ b/sdk/python/tests/test_keep_client.py
@@ -265,6 +265,34 @@ class KeepClientServiceTestCase(unittest.TestCase):
         self.assertEqual('100::1', service.hostname)
         self.assertEqual(10, service.port)
 
+    # test_get_timeout and test_put_timeout test that
+    # KeepClient.get and KeepClient.put use the appropriate timeouts
+    # when connected directly to a Keep server (i.e. non-proxy timeout)
+
+    def test_get_timeout(self):
+        api_client = self.mock_keep_services(('keep', 10, False, 'disk'))
+        keep_client = arvados.KeepClient(api_client=api_client)
+        force_timeout = [socket.timeout("timed out")]
+        with mock.patch('requests.get', side_effect=force_timeout) as mock_request:
+            with self.assertRaises(arvados.errors.KeepReadError):
+                keep_client.get('ffffffffffffffffffffffffffffffff')
+            self.assertTrue(mock_request.called)
+            self.assertEqual(
+                arvados.KeepClient.DEFAULT_TIMEOUT,
+                mock_request.call_args[1]['timeout'])
+
+    def test_put_timeout(self):
+        api_client = self.mock_keep_services(('keep', 10, False, 'disk'))
+        keep_client = arvados.KeepClient(api_client=api_client)
+        force_timeout = [socket.timeout("timed out")]
+        with mock.patch('requests.put', side_effect=force_timeout) as mock_request:
+            with self.assertRaises(arvados.errors.KeepWriteError):
+                keep_client.put('foo')
+            self.assertTrue(mock_request.called)
+            self.assertEqual(
+                arvados.KeepClient.DEFAULT_TIMEOUT,
+                mock_request.call_args[1]['timeout'])
+
 
 class KeepClientRetryTestMixin(object):
     # Testing with a local Keep store won't exercise the retry behavior.
@@ -327,6 +355,18 @@ class KeepClientRetryTestMixin(object):
         with tutil.mock_requestslib_responses(self.mock_method, self.DEFAULT_EXPECT, 500, 200):
             self.check_success()
 
+    def test_proxy_timeout(self):
+        # Force a timeout, verifying that the requests.get or
+        # requests.put method was called with the proxy_timeout
+        # setting rather than the default timeout.
+        force_timeout = [socket.timeout("timed out")]
+        with mock.patch(self.mock_method, side_effect=force_timeout) as mock_request:
+            self.check_exception()
+            self.assertTrue(mock_request.called)
+            self.assertEqual(
+                arvados.KeepClient.DEFAULT_PROXY_TIMEOUT,
+                mock_request.call_args[1]['timeout'])
+
 
 class KeepClientRetryGetTestMixin(KeepClientRetryTestMixin):
     mock_method = 'requests.get'

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list