3147: Make PySDK KeepClient.get and put retry_methods.

diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py
index 64d82ba563bcbf39c2562a5376ca7f54ca579165..95229760259f3c5020dba7eda8631300f9ab5fce 100644 (file)
--- a/sdk/python/arvados/keep.py
+++ b/sdk/python/arvados/keep.py
@@ -392,7 +392,8 @@ class KeepClient(object):
 
 
     def __init__(self, api_client=None, proxy=None, timeout=300,
 
 
     def __init__(self, api_client=None, proxy=None, timeout=300,

-                 api_token=None, local_store=None, block_cache=None):
+                 api_token=None, local_store=None, block_cache=None,
+                 num_retries=0):

         """Initialize a new KeepClient.
 
         Arguments:
         """Initialize a new KeepClient.
 
         Arguments:


@@ -416,6 +417,9 @@ class KeepClient(object):
           environment variable.  If you want to ensure KeepClient does not
           use local storage, pass in an empty string.  This is primarily
           intended to mock a server for testing.
           environment variable.  If you want to ensure KeepClient does not
           use local storage, pass in an empty string.  This is primarily
           intended to mock a server for testing.

+        * 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:
         """
         self.lock = threading.Lock()
         if proxy is None:


@@ -436,7 +440,7 @@ class KeepClient(object):
             self.put = self.local_store_put
         else:
             self.timeout = timeout
             self.put = self.local_store_put
         else:
             self.timeout = timeout

-
+            self.num_retries = num_retries

             if proxy:
                 if not proxy.endswith('/'):
                     proxy += '/'
             if proxy:
                 if not proxy.endswith('/'):
                     proxy += '/'


@@ -558,7 +562,8 @@ class KeepClient(object):
         else:
             return None
 
         else:
             return None
 

-    def get(self, loc_s, num_retries=0):
+    @retry.retry_method
+    def get(self, loc_s, num_retries=None):

         """Get data from Keep.
 
         This method fetches one or more blocks of data from Keep.  It
         """Get data from Keep.
 
         This method fetches one or more blocks of data from Keep.  It


@@ -575,7 +580,8 @@ class KeepClient(object):
           *each* Keep server if it returns temporary failures, with
           exponential backoff.  Note that, in each loop, the method may try
           to fetch data from every available Keep service, along with any
           *each* Keep server if it returns temporary failures, with
           exponential backoff.  Note that, in each loop, the method may try
           to fetch data from every available Keep service, along with any

-          that are named in location hints in the locator.  Default 0.
+          that are named in location hints in the locator.  The default value
+          is set when the KeepClient is initialized.

         """
         if ',' in loc_s:
             return ''.join(self.get(x) for x in loc_s.split(','))
         """
         if ',' in loc_s:
             return ''.join(self.get(x) for x in loc_s.split(','))


@@ -638,7 +644,8 @@ class KeepClient(object):
         else:
             raise arvados.errors.KeepReadError(loc_s)
 
         else:
             raise arvados.errors.KeepReadError(loc_s)
 

-    def put(self, data, copies=2, num_retries=0):
+    @retry.retry_method
+    def put(self, data, copies=2, num_retries=None):

         """Save data in Keep.
 
         This method will get a list of Keep services from the API server, and
         """Save data in Keep.
 
         This method will get a list of Keep services from the API server, and


@@ -653,7 +660,8 @@ class KeepClient(object):
           Default 2.
         * num_retries: The number of times to retry PUT requests to
           *each* Keep server if it returns temporary failures, with
           Default 2.
         * num_retries: The number of times to retry PUT requests to
           *each* Keep server if it returns temporary failures, with

-          exponential backoff.  Default 0.
+          exponential backoff.  The default value is set when the
+          KeepClient is initialized.

         """
         data_hash = hashlib.md5(data).hexdigest()
         if copies < 1:
         """
         data_hash = hashlib.md5(data).hexdigest()
         if copies < 1:

diff --git a/sdk/python/tests/test_collections.py b/sdk/python/tests/test_collections.py
index f6abe2b35ae2b46f38ba9491c6268ab6befd0e79..284854b31d6849d98605251d14a7a55128f14f7b 100644 (file)
--- a/sdk/python/tests/test_collections.py
+++ b/sdk/python/tests/test_collections.py
@@ -419,7 +419,7 @@ class ArvadosCollectionsTest(run_test_server.TestCaseWithServers,
 
 
     class MockKeep(object):
 
 
     class MockKeep(object):

-        def __init__(self, content):
+        def __init__(self, content, num_retries=0):

             self.content = content
 
         def get(self, locator):
             self.content = content
 
         def get(self, locator):

diff --git a/sdk/python/tests/test_keep_client.py b/sdk/python/tests/test_keep_client.py
index 9a01d86e1aa1f9e0423a72caf97107280242fd59..8cc22a6ee6ba27e336df7f88dd7ad67ab7ee817f 100644 (file)
--- a/sdk/python/tests/test_keep_client.py
+++ b/sdk/python/tests/test_keep_client.py
@@ -236,8 +236,13 @@ class KeepClientRetryTestMixin(object):
     TEST_DATA = 'testdata'
     TEST_LOCATOR = 'ef654c40ab4f1747fc699915d4f70902+8'
 
     TEST_DATA = 'testdata'
     TEST_LOCATOR = 'ef654c40ab4f1747fc699915d4f70902+8'
 

-    def new_client(self):
-        return arvados.KeepClient(proxy=self.PROXY_ADDR, local_store='')
+    def setUp(self):
+        self.client_kwargs = {'proxy': self.PROXY_ADDR, 'local_store': ''}
+
+    def new_client(self, **caller_kwargs):
+        kwargs = self.client_kwargs.copy()
+        kwargs.update(caller_kwargs)
+        return arvados.KeepClient(**kwargs)

 
     def run_method(self, *args, **kwargs):
         raise NotImplementedError("test subclasses must define run_method")
 
     def run_method(self, *args, **kwargs):
         raise NotImplementedError("test subclasses must define run_method")


@@ -272,6 +277,11 @@ class KeepClientRetryTestMixin(object):
         with tutil.mock_responses(self.DEFAULT_EXPECT, 500, 500, 200):
             self.check_exception(num_retries=1)
 
         with tutil.mock_responses(self.DEFAULT_EXPECT, 500, 500, 200):
             self.check_exception(num_retries=1)
 

+    def test_num_retries_instance_fallback(self):
+        self.client_kwargs['num_retries'] = 3
+        with tutil.mock_responses(self.DEFAULT_EXPECT, 500, 200):
+            self.check_success()
+

 
 @tutil.skip_sleep
 class KeepClientRetryGetTestCase(KeepClientRetryTestMixin, unittest.TestCase):
 
 @tutil.skip_sleep
 class KeepClientRetryGetTestCase(KeepClientRetryTestMixin, unittest.TestCase):