[ARVADOS] created: 3993ba462c113090bc2db79db30838c017a5aa3b
git at public.curoverse.com
git at public.curoverse.com
Wed Nov 5 18:12:28 EST 2014
at 3993ba462c113090bc2db79db30838c017a5aa3b (commit)
commit 3993ba462c113090bc2db79db30838c017a5aa3b
Author: Tim Pierce <twp at curoverse.com>
Date: Wed Nov 5 16:24:57 2014 -0500
3857: replaced httplib2 with 'requests'
Replaced httplib2 with the 'requests' library in modules that use it to
call Keep:
* arvados.collection
* arvados.keep
* arvados.retry
* arvados.stream
Updated unit tests and mocks appropriately.
diff --git a/sdk/python/arvados/collection.py b/sdk/python/arvados/collection.py
index 3a90d6d..334242b 100644
--- a/sdk/python/arvados/collection.py
+++ b/sdk/python/arvados/collection.py
@@ -1,6 +1,4 @@
import gflags
-import httplib
-import httplib2
import logging
import os
import pprint
diff --git a/sdk/python/arvados/keep.py b/sdk/python/arvados/keep.py
index 46bd1cb..d307363 100644
--- a/sdk/python/arvados/keep.py
+++ b/sdk/python/arvados/keep.py
@@ -1,6 +1,4 @@
import gflags
-import httplib
-import httplib2
import logging
import os
import pprint
@@ -21,6 +19,7 @@ import timer
import datetime
import ssl
import socket
+import requests
_logger = logging.getLogger('arvados.keep')
global_client_object = None
@@ -269,10 +268,10 @@ class KeepClient(object):
class KeepService(object):
# Make requests to a single Keep service, and track results.
- HTTP_ERRORS = (httplib2.HttpLib2Error, httplib.HTTPException,
+ HTTP_ERRORS = (requests.exceptions.RequestException,
socket.error, ssl.SSLError)
- def __init__(self, root, **headers):
+ def __init__(self, root, timeout=None, **headers):
self.root = root
self.last_result = None
self.success_flag = None
@@ -288,19 +287,19 @@ class KeepClient(object):
def last_status(self):
try:
- return int(self.last_result[0].status)
+ return self.last_result.status_code
except (AttributeError, IndexError, ValueError):
return None
- def get(self, http, locator):
- # http is an httplib2.Http object.
+ def get(self, locator, timeout=None):
# locator is a KeepLocator object.
url = self.root + str(locator)
_logger.debug("Request: GET %s", url)
try:
with timer.Timer() as t:
- result = http.request(url.encode('utf-8'), 'GET',
- headers=self.get_headers)
+ result = requests.get(url.encode('utf-8'),
+ headers=self.get_headers,
+ timeout=timeout)
except self.HTTP_ERRORS as e:
_logger.debug("Request fail: GET %s => %s: %s",
url, type(e), str(e))
@@ -308,7 +307,7 @@ class KeepClient(object):
else:
self.last_result = result
self.success_flag = retry.check_http_response_success(result)
- content = result[1]
+ content = result.content
_logger.info("%s response: %s bytes in %s msec (%.3f MiB/sec)",
self.last_status(), len(content), t.msecs,
(len(content)/(1024.0*1024))/t.secs)
@@ -320,12 +319,14 @@ class KeepClient(object):
url, resp_md5)
return None
- def put(self, http, hash_s, body):
+ def put(self, hash_s, body, timeout=None):
url = self.root + hash_s
_logger.debug("Request: PUT %s", url)
try:
- result = http.request(url.encode('utf-8'), 'PUT',
- headers=self.put_headers, body=body)
+ result = requests.put(url.encode('utf-8'),
+ data=body,
+ headers=self.put_headers,
+ timeout=timeout)
except self.HTTP_ERRORS as e:
_logger.debug("Request fail: PUT %s => %s: %s",
url, type(e), str(e))
@@ -366,12 +367,13 @@ class KeepClient(object):
str(threading.current_thread()),
self.args['data_hash'],
self.args['service_root'])
- h = httplib2.Http(timeout=self.args.get('timeout', None))
self._success = bool(self.service.put(
- h, self.args['data_hash'], self.args['data']))
+ self.args['data_hash'],
+ self.args['data'],
+ timeout=self.args.get('timeout', None)))
status = self.service.last_status()
if self._success:
- resp, body = self.service.last_result
+ result = self.service.last_result
_logger.debug("KeepWriterThread %s succeeded %s %s",
str(threading.current_thread()),
self.args['data_hash'],
@@ -381,14 +383,14 @@ class KeepClient(object):
# we're talking to a proxy or other backend that
# stores to multiple copies for us.
try:
- replicas_stored = int(resp['x-keep-replicas-stored'])
+ replicas_stored = int(result.headers['x-keep-replicas-stored'])
except (KeyError, ValueError):
replicas_stored = 1
- limiter.save_response(body.strip(), replicas_stored)
+ limiter.save_response(result.text.strip(), replicas_stored)
elif status is not None:
_logger.debug("Request fail: PUT %s => %s %s",
self.args['data_hash'], status,
- self.service.last_result[1])
+ self.service.last_result.text)
def __init__(self, api_client=None, proxy=None, timeout=300,
@@ -405,7 +407,8 @@ class KeepClient(object):
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.
+ 300. A tuple of two floats is interpreted as (connection_timeout,
+ read_timeout)
* 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
@@ -622,9 +625,8 @@ class KeepClient(object):
services_to_try = [roots_map[root]
for root in (local_roots + hint_roots)
if roots_map[root].usable()]
- http = httplib2.Http(timeout=self.timeout)
for keep_service in services_to_try:
- blob = keep_service.get(http, locator)
+ blob = keep_service.get(locator, timeout=self.timeout)
if blob is not None:
break
loop.save_result((blob, len(services_to_try)))
diff --git a/sdk/python/arvados/retry.py b/sdk/python/arvados/retry.py
index 3d2fc48..52a68fa 100644
--- a/sdk/python/arvados/retry.py
+++ b/sdk/python/arvados/retry.py
@@ -110,11 +110,11 @@ class RetryLoop(object):
def check_http_response_success(result):
- """Convert an httplib2 request result to a loop control flag.
+ """Convert a 'requests' response to a loop control flag.
- Pass this method the 2-tuple returned by httplib2.Http.request. It
- returns True if the response indicates success, None if it indicates
- temporary failure, and False otherwise. You can use this as the
+ Pass this method a requests.Response object. It returns True if
+ the response indicates success, None if it indicates temporary
+ failure, and False otherwise. You can use this as the
success_check for a RetryLoop.
Implementation details:
@@ -129,7 +129,7 @@ def check_http_response_success(result):
retry those requests verbatim.
"""
try:
- status = int(result[0].status)
+ status = result.status_code
except Exception:
return None
if status in _HTTP_SUCCESSES:
diff --git a/sdk/python/arvados/stream.py b/sdk/python/arvados/stream.py
index f5975b9..9b90cea 100644
--- a/sdk/python/arvados/stream.py
+++ b/sdk/python/arvados/stream.py
@@ -1,6 +1,4 @@
import gflags
-import httplib
-import httplib2
import os
import pprint
import sys
diff --git a/sdk/python/setup.py b/sdk/python/setup.py
index 64fba0c..e425290 100644
--- a/sdk/python/setup.py
+++ b/sdk/python/setup.py
@@ -45,6 +45,7 @@ setup(name='arvados-python-client',
'python-gflags',
'google-api-python-client',
'httplib2',
+ 'requests',
'urllib3',
'ws4py'
],
diff --git a/sdk/python/tests/arvados_testutil.py b/sdk/python/tests/arvados_testutil.py
index 0dbf9bc..d7d20e8 100644
--- a/sdk/python/tests/arvados_testutil.py
+++ b/sdk/python/tests/arvados_testutil.py
@@ -3,11 +3,13 @@
import errno
import httplib
import httplib2
+import io
import mock
import os
import shutil
import tempfile
import unittest
+import requests
# Use this hostname when you want to make sure the traffic will be
# instantly refused. 100::/64 is a dedicated black hole.
@@ -15,6 +17,8 @@ TEST_HOST = '100::'
skip_sleep = mock.patch('time.sleep', lambda n: None) # clown'll eat me
+# fake_httplib2_response and mock_responses
+# mock calls to httplib2.Http.request()
def fake_httplib2_response(code, **headers):
headers.update(status=str(code),
reason=httplib.responses.get(code, "Unknown Response"))
@@ -24,6 +28,28 @@ def mock_responses(body, *codes, **headers):
return mock.patch('httplib2.Http.request', side_effect=(
(fake_httplib2_response(code, **headers), body) for code in codes))
+# fake_requests_response, mock_get_responses and mock_put_responses
+# mock calls to requests.get() and requests.put()
+def fake_requests_response(code, body, **headers):
+ r = requests.Response()
+ r.status_code = code
+ r.reason = httplib.responses.get(code, "Unknown Response")
+ r.headers = headers
+ r.raw = io.BytesIO(body)
+ return r
+
+def mock_get_responses(body, *codes, **headers):
+ return mock.patch('requests.get', side_effect=(
+ fake_requests_response(code, body, **headers) for code in codes))
+
+def mock_put_responses(body, *codes, **headers):
+ return mock.patch('requests.put', side_effect=(
+ fake_requests_response(code, body, **headers) for code in codes))
+
+def mock_requestslib_responses(method, body, *codes, **headers):
+ return mock.patch(method, side_effect=(
+ fake_requests_response(code, body, **headers) for code in codes))
+
class ArvadosBaseTestCase(unittest.TestCase):
# This class provides common utility functions for our tests.
diff --git a/sdk/python/tests/test_collections.py b/sdk/python/tests/test_collections.py
index c4c7ca2..a179c2b 100644
--- a/sdk/python/tests/test_collections.py
+++ b/sdk/python/tests/test_collections.py
@@ -691,7 +691,7 @@ class CollectionTestMixin(object):
mock_method.return_value = body
else:
mock_method.side_effect = arvados.errors.ApiError(
- tutil.fake_httplib2_response(code), "{}")
+ tutil.fake_requests_response(code, None), "{}")
def mock_keep_services(self, api_mock, code, body):
self._mock_api_call(api_mock.keep_services().accessible, code, body)
@@ -736,7 +736,7 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
def test_locator_init(self):
client = self.api_client_mock(200)
# Ensure Keep will not return anything if asked.
- with tutil.mock_responses(None, 404):
+ with tutil.mock_get_responses(None, 404):
reader = arvados.CollectionReader(self.DEFAULT_DATA_HASH,
api_client=client)
self.assertEqual(self.DEFAULT_MANIFEST, reader.manifest_text())
@@ -746,7 +746,7 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
# been written to Keep.
client = self.api_client_mock(200)
self.mock_get_collection(client, 404, None)
- with tutil.mock_responses(self.DEFAULT_MANIFEST, 200):
+ with tutil.mock_get_responses(self.DEFAULT_MANIFEST, 200):
reader = arvados.CollectionReader(self.DEFAULT_DATA_HASH,
api_client=client)
self.assertEqual(self.DEFAULT_MANIFEST, reader.manifest_text())
@@ -756,7 +756,7 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
client = self.api_client_mock(404)
reader = arvados.CollectionReader(self.DEFAULT_UUID,
api_client=client)
- with tutil.mock_responses(self.DEFAULT_MANIFEST, 200):
+ with tutil.mock_get_responses(self.DEFAULT_MANIFEST, 200):
with self.assertRaises(arvados.errors.ApiError):
reader.manifest_text()
@@ -764,7 +764,7 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
# To verify that CollectionReader tries Keep first here, we
# mock API server to return the wrong data.
client = self.api_client_mock(200)
- with tutil.mock_responses(self.ALT_MANIFEST, 200):
+ with tutil.mock_get_responses(self.ALT_MANIFEST, 200):
self.assertEqual(
self.ALT_MANIFEST,
arvados.CollectionReader(
@@ -776,7 +776,7 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
client = self.api_client_mock(200)
reader = arvados.CollectionReader(self.DEFAULT_UUID, api_client=client,
num_retries=3)
- with tutil.mock_responses('foo', 500, 500, 200):
+ with tutil.mock_get_responses('foo', 500, 500, 200):
self.assertEqual('foo',
''.join(f.read(9) for f in reader.all_files()))
@@ -806,7 +806,7 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
class CollectionWriterTestCase(unittest.TestCase, CollectionTestMixin):
def mock_keep(self, body, *codes, **headers):
headers.setdefault('x-keep-replicas-stored', 2)
- return tutil.mock_responses(body, *codes, **headers)
+ return tutil.mock_put_responses(body, *codes, **headers)
def foo_writer(self, **kwargs):
api_client = self.api_client_mock()
diff --git a/sdk/python/tests/test_keep_client.py b/sdk/python/tests/test_keep_client.py
index d07d6e1..6a37a1b 100644
--- a/sdk/python/tests/test_keep_client.py
+++ b/sdk/python/tests/test_keep_client.py
@@ -303,33 +303,41 @@ class KeepClientRetryTestMixin(object):
self.assertRaises(error_class, self.run_method, *args, **kwargs)
def test_immediate_success(self):
- with tutil.mock_responses(self.DEFAULT_EXPECT, 200):
+ with tutil.mock_requestslib_responses(self.mock_method, self.DEFAULT_EXPECT, 200):
self.check_success()
def test_retry_then_success(self):
- with tutil.mock_responses(self.DEFAULT_EXPECT, 500, 200):
+ with tutil.mock_requestslib_responses(self.mock_method, self.DEFAULT_EXPECT, 500, 200):
self.check_success(num_retries=3)
def test_no_default_retry(self):
- with tutil.mock_responses(self.DEFAULT_EXPECT, 500, 200):
+ with tutil.mock_requestslib_responses(self.mock_method, self.DEFAULT_EXPECT, 500, 200):
self.check_exception()
def test_no_retry_after_permanent_error(self):
- with tutil.mock_responses(self.DEFAULT_EXPECT, 403, 200):
+ with tutil.mock_requestslib_responses(self.mock_method, self.DEFAULT_EXPECT, 403, 200):
self.check_exception(num_retries=3)
def test_error_after_retries_exhausted(self):
- with tutil.mock_responses(self.DEFAULT_EXPECT, 500, 500, 200):
+ with tutil.mock_requestslib_responses(self.mock_method, 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):
+ with tutil.mock_requestslib_responses(self.mock_method, self.DEFAULT_EXPECT, 500, 200):
self.check_success()
+class KeepClientRetryGetTestMixin(KeepClientRetryTestMixin):
+ mock_method = 'requests.get'
+
+
+class KeepClientRetryPutTestMixin(KeepClientRetryTestMixin):
+ mock_method = 'requests.put'
+
+
@tutil.skip_sleep
-class KeepClientRetryGetTestCase(KeepClientRetryTestMixin, unittest.TestCase):
+class KeepClientRetryGetTestCase(KeepClientRetryGetTestMixin, unittest.TestCase):
DEFAULT_EXPECT = KeepClientRetryTestMixin.TEST_DATA
DEFAULT_EXCEPTION = arvados.errors.KeepReadError
HINTED_LOCATOR = KeepClientRetryTestMixin.TEST_LOCATOR + '+K at xyzzy'
@@ -339,7 +347,7 @@ class KeepClientRetryGetTestCase(KeepClientRetryTestMixin, unittest.TestCase):
return self.new_client().get(locator, *args, **kwargs)
def test_specific_exception_when_not_found(self):
- with tutil.mock_responses(self.DEFAULT_EXPECT, 404, 200):
+ with tutil.mock_get_responses(self.DEFAULT_EXPECT, 404, 200):
self.check_exception(arvados.errors.NotFoundError, num_retries=3)
def test_general_exception_with_mixed_errors(self):
@@ -348,7 +356,7 @@ class KeepClientRetryGetTestCase(KeepClientRetryTestMixin, unittest.TestCase):
# This test rigs up 50/50 disagreement between two servers, and
# checks that it does not become a NotFoundError.
client = self.new_client()
- with tutil.mock_responses(self.DEFAULT_EXPECT, 404, 500):
+ with tutil.mock_get_responses(self.DEFAULT_EXPECT, 404, 500):
with self.assertRaises(arvados.errors.KeepReadError) as exc_check:
client.get(self.HINTED_LOCATOR)
self.assertNotIsInstance(
@@ -356,26 +364,26 @@ class KeepClientRetryGetTestCase(KeepClientRetryTestMixin, unittest.TestCase):
"mixed errors raised NotFoundError")
def test_hint_server_can_succeed_without_retries(self):
- with tutil.mock_responses(self.DEFAULT_EXPECT, 404, 200, 500):
+ with tutil.mock_get_responses(self.DEFAULT_EXPECT, 404, 200, 500):
self.check_success(locator=self.HINTED_LOCATOR)
def test_try_next_server_after_timeout(self):
side_effects = [
socket.timeout("timed out"),
- (tutil.fake_httplib2_response(200), self.DEFAULT_EXPECT)]
- with mock.patch('httplib2.Http.request',
+ tutil.fake_requests_response(200, self.DEFAULT_EXPECT)]
+ with mock.patch('requests.get',
side_effect=iter(side_effects)):
self.check_success(locator=self.HINTED_LOCATOR)
def test_retry_data_with_wrong_checksum(self):
- side_effects = ((tutil.fake_httplib2_response(200), s)
+ side_effects = (tutil.fake_requests_response(200, s)
for s in ['baddata', self.TEST_DATA])
- with mock.patch('httplib2.Http.request', side_effect=side_effects):
+ with mock.patch('requests.get', side_effect=side_effects):
self.check_success(locator=self.HINTED_LOCATOR)
@tutil.skip_sleep
-class KeepClientRetryPutTestCase(KeepClientRetryTestMixin, unittest.TestCase):
+class KeepClientRetryPutTestCase(KeepClientRetryPutTestMixin, unittest.TestCase):
DEFAULT_EXPECT = KeepClientRetryTestMixin.TEST_LOCATOR
DEFAULT_EXCEPTION = arvados.errors.KeepWriteError
@@ -384,5 +392,5 @@ class KeepClientRetryPutTestCase(KeepClientRetryTestMixin, unittest.TestCase):
return self.new_client().put(data, copies, *args, **kwargs)
def test_do_not_send_multiple_copies_to_same_server(self):
- with tutil.mock_responses(self.DEFAULT_EXPECT, 200):
+ with tutil.mock_put_responses(self.DEFAULT_EXPECT, 200):
self.check_exception(copies=2, num_retries=3)
diff --git a/sdk/python/tests/test_retry.py b/sdk/python/tests/test_retry.py
index 8c3916b..0c1110c 100644
--- a/sdk/python/tests/test_retry.py
+++ b/sdk/python/tests/test_retry.py
@@ -7,7 +7,7 @@ import arvados.errors as arv_error
import arvados.retry as arv_retry
import mock
-from arvados_testutil import fake_httplib2_response
+from arvados_testutil import fake_requests_response
class RetryLoopTestMixin(object):
@staticmethod
@@ -150,7 +150,7 @@ class RetryLoopBackoffTestCase(unittest.TestCase, RetryLoopTestMixin):
class CheckHTTPResponseSuccessTestCase(unittest.TestCase):
def results_map(self, *codes):
for code in codes:
- response = (fake_httplib2_response(code), None)
+ response = fake_requests_response(code, None)
yield code, arv_retry.check_http_response_success(response)
def check(assert_name):
diff --git a/sdk/python/tests/test_stream.py b/sdk/python/tests/test_stream.py
index 3970d67..db26aee 100644
--- a/sdk/python/tests/test_stream.py
+++ b/sdk/python/tests/test_stream.py
@@ -24,47 +24,47 @@ class StreamRetryTestMixin(object):
@tutil.skip_sleep
def test_success_without_retries(self):
reader = self.reader_for('bar_file')
- with tutil.mock_responses('bar', 200):
+ with tutil.mock_get_responses('bar', 200):
self.assertEqual('bar', self.read_for_test(reader, 3))
@tutil.skip_sleep
def test_read_no_default_retry(self):
reader = self.reader_for('user_agreement')
- with tutil.mock_responses('', 500):
+ with tutil.mock_get_responses('', 500):
with self.assertRaises(arvados.errors.KeepReadError):
self.read_for_test(reader, 10)
@tutil.skip_sleep
def test_read_with_instance_retries(self):
reader = self.reader_for('foo_file', num_retries=3)
- with tutil.mock_responses('foo', 500, 200):
+ with tutil.mock_get_responses('foo', 500, 200):
self.assertEqual('foo', self.read_for_test(reader, 3))
@tutil.skip_sleep
def test_read_with_method_retries(self):
reader = self.reader_for('foo_file')
- with tutil.mock_responses('foo', 500, 200):
+ with tutil.mock_get_responses('foo', 500, 200):
self.assertEqual('foo',
self.read_for_test(reader, 3, num_retries=3))
@tutil.skip_sleep
def test_read_instance_retries_exhausted(self):
reader = self.reader_for('bar_file', num_retries=3)
- with tutil.mock_responses('bar', 500, 500, 500, 500, 200):
+ with tutil.mock_get_responses('bar', 500, 500, 500, 500, 200):
with self.assertRaises(arvados.errors.KeepReadError):
self.read_for_test(reader, 3)
@tutil.skip_sleep
def test_read_method_retries_exhausted(self):
reader = self.reader_for('bar_file')
- with tutil.mock_responses('bar', 500, 500, 500, 500, 200):
+ with tutil.mock_get_responses('bar', 500, 500, 500, 500, 200):
with self.assertRaises(arvados.errors.KeepReadError):
self.read_for_test(reader, 3, num_retries=3)
@tutil.skip_sleep
def test_method_retries_take_precedence(self):
reader = self.reader_for('user_agreement', num_retries=10)
- with tutil.mock_responses('', 500, 500, 500, 200):
+ with tutil.mock_get_responses('', 500, 500, 500, 200):
with self.assertRaises(arvados.errors.KeepReadError):
self.read_for_test(reader, 10, num_retries=1)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list