From 420426e2faf27742138f990cca1880de3dabbc32 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Fri, 2 Sep 2016 10:50:50 -0700 Subject: [PATCH] Remove some unused features of gcloud.streaming. Towards #1998. --- gcloud/streaming/http_wrapper.py | 85 ++------------- gcloud/streaming/test_http_wrapper.py | 151 ++++++-------------------- gcloud/streaming/test_util.py | 4 +- gcloud/streaming/transfer.py | 4 +- gcloud/streaming/util.py | 11 +- 5 files changed, 48 insertions(+), 207 deletions(-) diff --git a/gcloud/streaming/http_wrapper.py b/gcloud/streaming/http_wrapper.py index 82d64b0455a9f..0eb856c97d946 100644 --- a/gcloud/streaming/http_wrapper.py +++ b/gcloud/streaming/http_wrapper.py @@ -35,6 +35,7 @@ from gcloud.streaming.util import calculate_wait_for_retry +_REDIRECTIONS = 5 # 308 and 429 don't have names in httplib. RESUME_INCOMPLETE = 308 TOO_MANY_REQUESTS = 429 @@ -62,27 +63,6 @@ ) -class _ExceptionRetryArgs( - collections.namedtuple( - '_ExceptionRetryArgs', - ['http', 'http_request', 'exc', 'num_retries', 'max_retry_wait'])): - """Bundle of information for retriable exceptions. - - :type http: :class:`httplib2.Http` (or conforming alternative) - :param http: instance used to perform requests. - - :type http_request: :class:`Request` - :param http_request: the request whose response was a retriable error. - - :type exc: :class:`Exception` subclass - :param exc: the exception being raised. - - :type num_retries: integer - :param num_retries: Number of retries consumed; used for exponential - backoff. - """ - - @contextlib.contextmanager def _httplib2_debug_level(http_request, level, http=None): """Temporarily change the value of httplib2.debuglevel, if necessary. @@ -328,8 +308,7 @@ def _reset_http_connections(http): del http.connections[conn_key] -def _make_api_request_no_retry(http, http_request, redirections=5, - check_response_func=_check_response): +def _make_api_request_no_retry(http, http_request, redirections=_REDIRECTIONS): """Send an HTTP request via the given http instance. This wrapper exists to handle translation between the plain httplib2 @@ -344,9 +323,6 @@ def _make_api_request_no_retry(http, http_request, redirections=5, :type redirections: integer :param redirections: Number of redirects to follow. - :type check_response_func: function taking (response, content, url). - :param check_response_func: Function to validate the HTTP response. - :rtype: :class:`Response` :returns: an object representing the server's response @@ -374,16 +350,12 @@ def _make_api_request_no_retry(http, http_request, redirections=5, raise RequestError() response = Response(info, content, http_request.url) - check_response_func(response) + _check_response(response) return response -def make_api_request(http, http_request, - retries=7, - max_retry_wait=60, - redirections=5, - check_response_func=_check_response, - wo_retry_func=_make_api_request_no_retry): +def make_api_request(http, http_request, retries=7, + redirections=_REDIRECTIONS): """Send an HTTP request via the given http, performing error/retry handling. :type http: :class:`httplib2.Http` @@ -396,19 +368,9 @@ def make_api_request(http, http_request, :param retries: Number of retries to attempt on retryable responses (such as 429 or 5XX). - :type max_retry_wait: integer - :param max_retry_wait: Maximum number of seconds to wait when retrying. - :type redirections: integer :param redirections: Number of redirects to follow. - :type check_response_func: function taking (response, content, url). - :param check_response_func: Function to validate the HTTP response. - - :type wo_retry_func: function taking - (http, request, redirections, check_response_func) - :param wo_retry_func: Function to make HTTP request without retries. - :rtype: :class:`Response` :returns: an object representing the server's response. @@ -418,48 +380,17 @@ def make_api_request(http, http_request, retry = 0 while True: try: - return wo_retry_func( - http, http_request, redirections=redirections, - check_response_func=check_response_func) + return _make_api_request_no_retry(http, http_request, + redirections=redirections) except _RETRYABLE_EXCEPTIONS as exc: retry += 1 if retry >= retries: raise retry_after = getattr(exc, 'retry_after', None) if retry_after is None: - retry_after = calculate_wait_for_retry(retry, max_retry_wait) + retry_after = calculate_wait_for_retry(retry) _reset_http_connections(http) logging.debug('Retrying request to url %s after exception %s', http_request.url, type(exc).__name__) time.sleep(retry_after) - - -_HTTP_FACTORIES = [] - - -def _register_http_factory(factory): - """Register a custom HTTP factory. - - :type factory: callable taking keyword arguments, returning an Http - instance (or an instance implementing the same API). - :param factory: the new factory (it may return ``None`` to defer to - a later factory or the default). - """ - _HTTP_FACTORIES.append(factory) - - -def get_http(**kwds): - """Construct an Http instance. - - :type kwds: dict - :param kwds: keyword arguments to pass to factories. - - :rtype: :class:`httplib2.Http` (or a workalike) - :returns: The HTTP object created. - """ - for factory in _HTTP_FACTORIES: - http = factory(**kwds) - if http is not None: - return http - return httplib2.Http(**kwds) diff --git a/gcloud/streaming/test_http_wrapper.py b/gcloud/streaming/test_http_wrapper.py index d1bdd86172928..70c8eaa86d1a1 100644 --- a/gcloud/streaming/test_http_wrapper.py +++ b/gcloud/streaming/test_http_wrapper.py @@ -277,35 +277,16 @@ def test_defaults_wo_connections(self): _httplib2 = _Dummy(debuglevel=1) _request = _Request() _checked = [] - with _Monkey(MUT, httplib2=_httplib2): - response = self._callFUT(_http, _request, - check_response_func=_checked.append) - self.assertTrue(isinstance(response, MUT.Response)) - self.assertEqual(response.info, INFO) - self.assertEqual(response.content, CONTENT) - self.assertEqual(response.request_url, _request.url) - self.assertEqual(_checked, [response]) - self._verify_requested(_http, _request) + with _Monkey(MUT, httplib2=_httplib2, + _check_response=_checked.append): + response = self._callFUT(_http, _request) - def test_w_explicit_redirections(self): - from gcloud._testing import _Monkey - from gcloud.streaming import http_wrapper as MUT - INFO = {'status': '200'} - CONTENT = 'CONTENT' - _http = _Http((INFO, CONTENT)) - _httplib2 = _Dummy(debuglevel=1) - _request = _Request() - _checked = [] - with _Monkey(MUT, httplib2=_httplib2): - response = self._callFUT(_http, _request, - redirections=10, - check_response_func=_checked.append) self.assertTrue(isinstance(response, MUT.Response)) self.assertEqual(response.info, INFO) self.assertEqual(response.content, CONTENT) self.assertEqual(response.request_url, _request.url) self.assertEqual(_checked, [response]) - self._verify_requested(_http, _request, redirections=10) + self._verify_requested(_http, _request) def test_w_http_connections_miss(self): from gcloud._testing import _Monkey @@ -318,9 +299,10 @@ def test_w_http_connections_miss(self): _httplib2 = _Dummy(debuglevel=1) _request = _Request() _checked = [] - with _Monkey(MUT, httplib2=_httplib2): - response = self._callFUT(_http, _request, - check_response_func=_checked.append) + with _Monkey(MUT, httplib2=_httplib2, + _check_response=_checked.append): + response = self._callFUT(_http, _request) + self.assertTrue(isinstance(response, MUT.Response)) self.assertEqual(response.info, INFO) self.assertEqual(response.content, CONTENT) @@ -339,9 +321,10 @@ def test_w_http_connections_hit(self): _httplib2 = _Dummy(debuglevel=1) _request = _Request() _checked = [] - with _Monkey(MUT, httplib2=_httplib2): - response = self._callFUT(_http, _request, - check_response_func=_checked.append) + with _Monkey(MUT, httplib2=_httplib2, + _check_response=_checked.append): + response = self._callFUT(_http, _request) + self.assertTrue(isinstance(response, MUT.Response)) self.assertEqual(response.info, INFO) self.assertEqual(response.content, CONTENT) @@ -373,6 +356,9 @@ def _callFUT(self, *args, **kw): return make_api_request(*args, **kw) def test_wo_exception(self): + from gcloud._testing import _Monkey + from gcloud.streaming import http_wrapper as MUT + HTTP, REQUEST, RESPONSE = object(), object(), object() _created, _checked = [], [] @@ -380,23 +366,22 @@ def _wo_exception(*args, **kw): _created.append((args, kw)) return RESPONSE - response = self._callFUT(HTTP, REQUEST, - wo_retry_func=_wo_exception, - check_response_func=_checked.append) + with _Monkey(MUT, _make_api_request_no_retry=_wo_exception, + _check_response=_checked.append): + response = self._callFUT(HTTP, REQUEST) self.assertTrue(response is RESPONSE) - expected_kw = { - 'redirections': 5, - 'check_response_func': _checked.append, - } + expected_kw = {'redirections': MUT._REDIRECTIONS} self.assertEqual(_created, [((HTTP, REQUEST), expected_kw)]) self.assertEqual(_checked, []) # not called by '_wo_exception' def test_w_exceptions_lt_max_retries(self): + from gcloud._testing import _Monkey from gcloud.streaming.exceptions import RetryAfterError + from gcloud.streaming import http_wrapper as MUT + HTTP, RESPONSE = object(), object() REQUEST = _Request() - WAIT = 10, _created, _checked = [], [] _counter = [None] * 4 @@ -407,18 +392,13 @@ def _wo_exception(*args, **kw): raise RetryAfterError(RESPONSE, '', REQUEST.url, 0.1) return RESPONSE - response = self._callFUT(HTTP, REQUEST, - retries=5, - max_retry_wait=WAIT, - wo_retry_func=_wo_exception, - check_response_func=_checked.append) + with _Monkey(MUT, _make_api_request_no_retry=_wo_exception, + _check_response=_checked.append): + response = self._callFUT(HTTP, REQUEST, retries=5) self.assertTrue(response is RESPONSE) self.assertEqual(len(_created), 5) - expected_kw = { - 'redirections': 5, - 'check_response_func': _checked.append, - } + expected_kw = {'redirections': MUT._REDIRECTIONS} for attempt in _created: self.assertEqual(attempt, ((HTTP, REQUEST), expected_kw)) self.assertEqual(_checked, []) # not called by '_wo_exception' @@ -428,94 +408,25 @@ def test_w_exceptions_gt_max_retries(self): from gcloud.streaming import http_wrapper as MUT HTTP = object() REQUEST = _Request() - WAIT = 10, _created, _checked = [], [] def _wo_exception(*args, **kw): _created.append((args, kw)) raise ValueError('Retryable') - with _Monkey(MUT, calculate_wait_for_retry=lambda *ignored: 0.1): + with _Monkey(MUT, calculate_wait_for_retry=lambda *ignored: 0.1, + _make_api_request_no_retry=_wo_exception, + _check_response=_checked.append): with self.assertRaises(ValueError): - self._callFUT(HTTP, REQUEST, - retries=3, - max_retry_wait=WAIT, - wo_retry_func=_wo_exception, - check_response_func=_checked.append) + self._callFUT(HTTP, REQUEST, retries=3) self.assertEqual(len(_created), 3) - expected_kw = { - 'redirections': 5, - 'check_response_func': _checked.append, - } + expected_kw = {'redirections': MUT._REDIRECTIONS} for attempt in _created: self.assertEqual(attempt, ((HTTP, REQUEST), expected_kw)) self.assertEqual(_checked, []) # not called by '_wo_exception' -class Test__register_http_factory(unittest.TestCase): - - def _callFUT(self, *args, **kw): - from gcloud.streaming.http_wrapper import _register_http_factory - return _register_http_factory(*args, **kw) - - def test_it(self): - from gcloud._testing import _Monkey - from gcloud.streaming import http_wrapper as MUT - _factories = [] - - FACTORY = object() - - with _Monkey(MUT, _HTTP_FACTORIES=_factories): - self._callFUT(FACTORY) - self.assertEqual(_factories, [FACTORY]) - - -class Test_get_http(unittest.TestCase): - - def _callFUT(self, *args, **kw): - from gcloud.streaming.http_wrapper import get_http - return get_http(*args, **kw) - - def test_wo_registered_factories(self): - from httplib2 import Http - from gcloud._testing import _Monkey - from gcloud.streaming import http_wrapper as MUT - _factories = [] - - with _Monkey(MUT, _HTTP_FACTORIES=_factories): - http = self._callFUT() - - self.assertTrue(isinstance(http, Http)) - - def test_w_registered_factories(self): - from gcloud._testing import _Monkey - from gcloud.streaming import http_wrapper as MUT - - FOUND = object() - - _misses = [] - - def _miss(**kw): - _misses.append(kw) - return None - - _hits = [] - - def _hit(**kw): - _hits.append(kw) - return FOUND - - _factories = [_miss, _hit] - - with _Monkey(MUT, _HTTP_FACTORIES=_factories): - http = self._callFUT(foo='bar') - - self.assertTrue(http is FOUND) - self.assertEqual(_misses, [{'foo': 'bar'}]) - self.assertEqual(_hits, [{'foo': 'bar'}]) - - class _Dummy(object): def __init__(self, **kw): self.__dict__.update(kw) diff --git a/gcloud/streaming/test_util.py b/gcloud/streaming/test_util.py index 560eba4dd1d63..c0b6ffbfb07b8 100644 --- a/gcloud/streaming/test_util.py +++ b/gcloud/streaming/test_util.py @@ -11,13 +11,13 @@ def test_w_negative_jitter_lt_max_wait(self): import random from gcloud._testing import _Monkey with _Monkey(random, uniform=lambda lower, upper: lower): - self.assertEqual(self._callFUT(1, 60), 1.5) + self.assertEqual(self._callFUT(1), 1.5) def test_w_positive_jitter_gt_max_wait(self): import random from gcloud._testing import _Monkey with _Monkey(random, uniform=lambda lower, upper: upper): - self.assertEqual(self._callFUT(4, 10), 10) + self.assertEqual(self._callFUT(4), 20) class Test_acceptable_mime_type(unittest.TestCase): diff --git a/gcloud/streaming/transfer.py b/gcloud/streaming/transfer.py index bc11017dc7c0f..360075622d791 100644 --- a/gcloud/streaming/transfer.py +++ b/gcloud/streaming/transfer.py @@ -22,6 +22,7 @@ import mimetypes import os +import httplib2 import six from six.moves import http_client @@ -31,7 +32,6 @@ from gcloud.streaming.exceptions import HttpError from gcloud.streaming.exceptions import TransferInvalidError from gcloud.streaming.exceptions import TransferRetryError -from gcloud.streaming.http_wrapper import get_http from gcloud.streaming.http_wrapper import make_api_request from gcloud.streaming.http_wrapper import Request from gcloud.streaming.http_wrapper import RESUME_INCOMPLETE @@ -184,7 +184,7 @@ def _initialize(self, http, url): """ self._ensure_uninitialized() if self.http is None: - self._http = http or get_http() + self._http = http or httplib2.Http() self._url = url @property diff --git a/gcloud/streaming/util.py b/gcloud/streaming/util.py index adba960615cdf..40d2b1749f7f2 100644 --- a/gcloud/streaming/util.py +++ b/gcloud/streaming/util.py @@ -17,7 +17,10 @@ import random -def calculate_wait_for_retry(retry_attempt, max_wait=60): +_MAX_RETRY_WAIT = 60 + + +def calculate_wait_for_retry(retry_attempt): """Calculate the amount of time to wait before a retry attempt. Wait time grows exponentially with the number of attempts. A @@ -27,17 +30,13 @@ def calculate_wait_for_retry(retry_attempt, max_wait=60): :type retry_attempt: integer :param retry_attempt: Retry attempt counter. - :type max_wait: integer - :param max_wait: Upper bound for wait time [seconds]. - :rtype: integer :returns: Number of seconds to wait before retrying request. """ - wait_time = 2 ** retry_attempt max_jitter = wait_time / 4.0 wait_time += random.uniform(-max_jitter, max_jitter) - return max(1, min(wait_time, max_wait)) + return max(1, min(wait_time, _MAX_RETRY_WAIT)) def acceptable_mime_type(accept_patterns, mime_type):