diff --git a/.travis.yml b/.travis.yml index 267e2a6..5df20ee 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,7 +11,7 @@ matrix: # Disabled until gRPC supports PyPy. # - python: pypy # env: TOXENV=pypy - - python: 3.6 + - python: 3.5 env: TOXENV=lint - python: 3.6 env: TOXENV=docs diff --git a/google/gax/retry.py b/google/gax/retry.py index b7ed436..729b12f 100644 --- a/google/gax/retry.py +++ b/google/gax/retry.py @@ -32,9 +32,13 @@ from __future__ import absolute_import, division import random +import sys import time -from google.gax import config, errors +import six + +from google.gax import config +from google.gax import errors _MILLIS_PER_SECOND = 1000 @@ -101,8 +105,6 @@ def inner(*args): by the options in ``retry``. """ delay = retry_options.backoff_settings.initial_retry_delay_millis - exc = errors.RetryError('Retry total timeout exceeded before any' - 'response was received') if has_timeout_settings: timeout = ( retry_options.backoff_settings.initial_rpc_timeout_millis / @@ -118,16 +120,14 @@ def inner(*args): try: to_call = add_timeout_arg(a_func, timeout, **kwargs) return to_call(*args) - except Exception as exception: # pylint: disable=broad-except + except tuple(config.API_ERRORS) as exception: code = config.exc_to_code(exception) - if code not in retry_options.retry_codes: - raise errors.RetryError( - 'Exception occurred in retry method that was not' - ' classified as transient', exception) - # pylint: disable=redefined-variable-type - exc = errors.RetryError( - 'Retry total timeout exceeded with exception', exception) + # If this is an exception that we do not know for sure should + # be retried, wrap it in the way that we would any standard + # error. + if code not in retry_options.retry_codes: + raise errors.create_error(str(exception), cause=exception) # Sleep a random number which will, on average, equal the # expected delay. @@ -140,6 +140,16 @@ def inner(*args): timeout = min( timeout * timeout_mult, max_timeout, deadline - now) - raise exc + # If we have passed our deadline, raise the exception. + if deadline is not None and now >= deadline: + exc = errors.RetryError( + 'Retry timeout exceeded; exception: %s' % exception, + cause=exception, + ) + six.reraise(errors.RetryError, exc, sys.exc_info()[2]) + + # If we got here, then we timed out and never got a response at all. + raise errors.RetryError('Retry total timeout exceeded before any' + 'response was received.') return inner diff --git a/setup.py b/setup.py index 1a1f12b..e14b762 100644 --- a/setup.py +++ b/setup.py @@ -41,6 +41,7 @@ 'oauth2client>=2.0.0, <4.0dev', 'ply==3.8', 'protobuf>=3.0.0, <4.0dev', + 'six>=1.9.0', ] with open('README.rst', 'r') as readme: diff --git a/tests/test_api_callable.py b/tests/test_api_callable.py index f0caed2..e5433cb 100644 --- a/tests/test_api_callable.py +++ b/tests/test_api_callable.py @@ -43,7 +43,8 @@ __version__ as GAX_VERSION, _CallSettings, api_callable, BackoffSettings, BundleDescriptor, BundleOptions, bundling, CallOptions, INITIAL_PAGE, PageDescriptor, RetryOptions) -from google.gax.errors import GaxError + +from tests.utils import errors # pylint: disable=no-member GRPC_VERSION = pkg_resources.get_distribution('grpcio').version @@ -106,9 +107,6 @@ 'code_c': Exception} -_FAKE_STATUS_CODE_1 = object() - - class CustomException(Exception): def __init__(self, msg, code): super(CustomException, self).__init__(msg) @@ -142,24 +140,26 @@ def test_call_kwargs(self): 'updated') @mock.patch('time.time') - @mock.patch('google.gax.config.exc_to_code') - def test_retry(self, mock_exc_to_code, mock_time): - mock_exc_to_code.side_effect = lambda e: e.code + def test_retry(self, mock_time): to_attempt = 3 retry = RetryOptions( - [_FAKE_STATUS_CODE_1], - BackoffSettings(0, 0, 0, 0, 0, 0, 1)) - - # Succeeds on the to_attempt'th call, and never again afterward - mock_call = mock.Mock() - mock_call.side_effect = ([CustomException('', _FAKE_STATUS_CODE_1)] * - (to_attempt - 1) + [mock.DEFAULT]) - mock_call.return_value = 1729 + ['BOGUS_STATUS_CODE'], + BackoffSettings(0, 0, 0, 0, 0, 0, 1), + ) + + # Define the exception to be raised on every attempt before the + # last one, and the result for the last attempt. + to_attempt = 3 + exc = errors.MockGrpcException(code='BOGUS_STATUS_CODE') + mock_func = mock.Mock() + mock_func.side_effect = [exc] * (to_attempt - 1) + [mock.DEFAULT] + mock_func.return_value = 1729 + mock_time.return_value = 0 - settings = _CallSettings(timeout=0, retry=retry) - my_callable = api_callable.create_api_call(mock_call, settings) + settings = _CallSettings(timeout=None, retry=retry) + my_callable = api_callable.create_api_call(mock_func, settings) self.assertEqual(my_callable(None), 1729) - self.assertEqual(mock_call.call_count, to_attempt) + self.assertEqual(mock_func.call_count, to_attempt) def test_page_streaming(self): # A mock grpc function that page streams a list of consecutive @@ -388,7 +388,7 @@ def other_error_func(*dummy_args, **dummy_kwargs): gax_error_callable = api_callable.create_api_call( abortion_error_func, _CallSettings()) - self.assertRaises(GaxError, gax_error_callable, None) + self.assertRaises(errors.GaxError, gax_error_callable, None) other_error_callable = api_callable.create_api_call( other_error_func, _CallSettings()) diff --git a/tests/test_retry.py b/tests/test_retry.py index 5b12df0..4e396f6 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -32,149 +32,147 @@ from __future__ import absolute_import, division +import unittest + import mock -import unittest2 -from google.gax import BackoffSettings, errors, retry, RetryOptions +from google.gax import BackoffSettings +from google.gax import retry +from google.gax import RetryOptions -_MILLIS_PER_SEC = 1000 +from tests.utils import errors -_FAKE_STATUS_CODE_1 = object() - +_MILLIS_PER_SEC = 1000 +_FAKE_STATUS_CODE_1 = object() _FAKE_STATUS_CODE_2 = object() -class CustomException(Exception): - def __init__(self, msg, code): - super(CustomException, self).__init__(msg) - self.code = code - - -class TestRetry(unittest2.TestCase): - - @mock.patch('google.gax.config.exc_to_code') +class TestRetry(unittest.TestCase): @mock.patch('time.time') - def test_retryable_without_timeout(self, mock_time, mock_exc_to_code): + def test_retryable_without_timeout(self, mock_time): mock_time.return_value = 0 - mock_exc_to_code.side_effect = lambda e: e.code + # Define the exception to be raised on every attempt before the + # last one, and the result for the last attempt. to_attempt = 3 - mock_call = mock.Mock() - mock_call.side_effect = ([CustomException('', _FAKE_STATUS_CODE_1)] * - (to_attempt - 1) + [mock.DEFAULT]) - mock_call.return_value = 1729 + exc = errors.MockGrpcException(code=_FAKE_STATUS_CODE_1) + mock_func = mock.Mock() + mock_func.side_effect = [exc] * (to_attempt - 1) + [mock.DEFAULT] + mock_func.return_value = 1729 retry_options = RetryOptions( [_FAKE_STATUS_CODE_1], - BackoffSettings(0, 0, 0, None, None, None, None)) + BackoffSettings(0, 0, 0, None, None, None, None), + ) - my_callable = retry.retryable(mock_call, retry_options) + my_callable = retry.retryable(mock_func, retry_options) + result = my_callable(None) - self.assertEqual(my_callable(None), 1729) - self.assertEqual(to_attempt, mock_call.call_count) + self.assertEqual(result, 1729) + self.assertEqual(to_attempt, mock_func.call_count) - @mock.patch('google.gax.config.exc_to_code') @mock.patch('time.time') - def test_retryable_with_timeout(self, mock_time, mock_exc_to_code): + def test_retryable_with_timeout(self, mock_time): mock_time.return_value = 1 - mock_exc_to_code.side_effect = lambda e: e.code - mock_call = mock.Mock() - mock_call.side_effect = [CustomException('', _FAKE_STATUS_CODE_1), - mock.DEFAULT] - mock_call.return_value = 1729 + mock_func = mock.Mock() + mock_func.side_effect = [ + errors.MockGrpcException(code=_FAKE_STATUS_CODE_1), + mock.DEFAULT, + ] + mock_func.return_value = 1729 retry_options = RetryOptions( [_FAKE_STATUS_CODE_1], - BackoffSettings(0, 0, 0, 0, 0, 0, 0)) + BackoffSettings(0, 0, 0, 0, 0, 0, 0), + ) - my_callable = retry.retryable(mock_call, retry_options) + my_callable = retry.retryable(mock_func, retry_options) self.assertRaises(errors.RetryError, my_callable) - self.assertEqual(0, mock_call.call_count) + self.assertEqual(0, mock_func.call_count) - @mock.patch('google.gax.config.exc_to_code') @mock.patch('time.time') - def test_retryable_when_no_codes(self, mock_time, mock_exc_to_code): + def test_retryable_when_no_codes(self, mock_time): mock_time.return_value = 0 - mock_exc_to_code.side_effect = lambda e: e.code - - mock_call = mock.Mock() - mock_call.side_effect = [CustomException('', _FAKE_STATUS_CODE_1), - mock.DEFAULT] - mock_call.return_value = 1729 + # Set up the mock function to raise an exception that is *not* + # an expected code. + mock_func = mock.Mock() + mock_func.side_effect = [ + errors.MockGrpcException(code=_FAKE_STATUS_CODE_2), + mock.DEFAULT, + ] + mock_func.return_value = 1729 + + # Set the retry options not to actually honor any codes + # (thus, our code is not in the list). retry_options = RetryOptions( [], - BackoffSettings(0, 0, 0, 0, 0, 0, 1)) + BackoffSettings(0, 0, 0, 0, 0, 0, 1), + ) - my_callable = retry.retryable(mock_call, retry_options) - - try: + # Create the callable and establish that we get a GaxError. + my_callable = retry.retryable(mock_func, retry_options) + with self.assertRaises(errors.GaxError): my_callable(None) - self.fail('Should not have been reached') - except errors.RetryError as exc: - self.assertIsInstance(exc.cause, CustomException) - self.assertEqual(1, mock_call.call_count) + # The actual retryable function should have been called exactly once. + mock_func.assert_called_once() - @mock.patch('google.gax.config.exc_to_code') @mock.patch('time.time') - def test_retryable_aborts_on_unexpected_exception( - self, mock_time, mock_exc_to_code): + def test_retryable_aborts_on_unexpected_exception(self, mock_time): mock_time.return_value = 0 - mock_exc_to_code.side_effect = lambda e: e.code - mock_call = mock.Mock() - mock_call.side_effect = [CustomException('', _FAKE_STATUS_CODE_2), - mock.DEFAULT] - mock_call.return_value = 1729 + # Set up the mock function to raise an exception that should be + # bubbled up (because it is not recognized). + mock_func = mock.Mock() + mock_func.side_effect = [ + ValueError('bogus'), + mock.DEFAULT, + ] + mock_func.return_value = 1729 retry_options = RetryOptions( [_FAKE_STATUS_CODE_1], - BackoffSettings(0, 0, 0, 0, 0, 0, 1)) - - my_callable = retry.retryable(mock_call, retry_options) + BackoffSettings(0, 0, 0, 0, 0, 0, 1), + ) + my_callable = retry.retryable(mock_func, retry_options) - try: + # Establish that the custom exception is bubbled up (not wrapped), and + # that the retryable function was called only once, not twice. + with self.assertRaises(ValueError): my_callable(None) - self.fail('Should not have been reached') - except errors.RetryError as exc: - self.assertIsInstance(exc.cause, CustomException) - - self.assertEqual(1, mock_call.call_count) + mock_func.assert_called_once() - @mock.patch('google.gax.config.exc_to_code') @mock.patch('time.sleep') @mock.patch('time.time') - def test_retryable_exponential_backoff( - self, mock_time, mock_sleep, mock_exc_to_code): + def test_retryable_exponential_backoff(self, mock_time, mock_sleep): def incr_time(secs): mock_time.return_value += secs def api_call(timeout): incr_time(timeout) - raise CustomException(str(timeout), _FAKE_STATUS_CODE_1) + raise errors.MockGrpcException(str(timeout), _FAKE_STATUS_CODE_1) mock_time.return_value = 0 mock_sleep.side_effect = incr_time - mock_exc_to_code.side_effect = lambda e: e.code - mock_call = mock.Mock() - mock_call.side_effect = api_call + mock_func = mock.Mock() + mock_func.side_effect = api_call params = BackoffSettings(3, 2, 24, 5, 2, 80, 2500) retry_options = RetryOptions([_FAKE_STATUS_CODE_1], params) - my_callable = retry.retryable(mock_call, retry_options) + my_callable = retry.retryable(mock_func, retry_options) try: my_callable() self.fail('Should not have been reached') except errors.RetryError as exc: - self.assertIsInstance(exc.cause, CustomException) + self.assertIsInstance(exc.cause, errors.MockGrpcException) self.assertGreaterEqual(mock_time(), params.total_timeout_millis / _MILLIS_PER_SEC) @@ -182,8 +180,8 @@ def api_call(timeout): # Very rough bounds calls_lower_bound = params.total_timeout_millis / ( params.max_retry_delay_millis + params.max_rpc_timeout_millis) - self.assertGreater(mock_call.call_count, calls_lower_bound) + self.assertGreater(mock_func.call_count, calls_lower_bound) calls_upper_bound = (params.total_timeout_millis / params.initial_retry_delay_millis) - self.assertLess(mock_call.call_count, calls_upper_bound) + self.assertLess(mock_func.call_count, calls_upper_bound) diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/utils/errors.py b/tests/utils/errors.py new file mode 100644 index 0000000..7eaf72d --- /dev/null +++ b/tests/utils/errors.py @@ -0,0 +1,44 @@ +# Copyright 2017, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +from __future__ import absolute_import + +from google.gax import config +from google.gax.errors import GaxError, RetryError + +__all__ = ['GaxError', 'MockGrpcException', 'RetryError'] + + +class MockGrpcException(config.API_ERRORS[0]): + def __init__(self, msg='', code=None): + super(MockGrpcException, self).__init__(msg) + self._code = code + + def code(self): + return self._code diff --git a/tox.ini b/tox.ini index 8d793d3..768a675 100644 --- a/tox.ini +++ b/tox.ini @@ -14,7 +14,7 @@ commands = coverage report --show-missing --fail-under=99 [testenv:lint] -basepython = python3.6 +basepython = python3.5 commands = python setup.py check --metadata --restructuredtext --strict flake8 \