feat(core): make the last retry happen at deadline

This commit changes Retry instances in a way that the last sleep period
is shortened so that its end matches at the given deadline. This
prevents the last sleep period to last way beyond the deadline.

Author: plamut

api_core/google/api_core/retry.py

@@ -141,9 +141,7 @@ def exponential_sleep_generator(initial, maximum, multiplier=_DEFAULT_DELAY_MULT
         delay = delay * multiplier
 
 
-def retry_target(
-    target, predicate, sleep_generator, deadline, on_error=None, strict_deadline=False
-):
+def retry_target(target, predicate, sleep_generator, deadline, on_error=None):
     """Call a function and retry if it fails.
 
     This is the lowest-level retry helper. Generally, you'll use the
@@ -157,13 +155,12 @@ def retry_target(
             It should return True to retry or False otherwise.
         sleep_generator (Iterable[float]): An infinite iterator that determines
             how long to sleep between retries.
-        deadline (float): How long to keep retrying the target.
+        deadline (float): How long to keep retrying the target. The last sleep
+            period is shortened as necessary, so that the last retry runs at
+            ``deadline`` (and not considerably beyond it).
         on_error (Callable[Exception]): A function to call while processing a
             retryable exception.  Any error raised by this function will *not*
             be caught.
-        strict_deadline (bool): If :data:`True`, the last retry will run at
-            ``deadline``, shortening the last sleep interval as necessary.
-            Defaults to :data:`False`.
 
     Returns:
         Any: the return value of the target function.
@@ -208,7 +205,7 @@ def retry_target(
                     ),
                     last_exc,
                 )
-            elif strict_deadline:
+            else:
                 time_to_deadline = (deadline_datetime - now).total_seconds()
                 sleep = min(time_to_deadline, sleep)
 
@@ -237,10 +234,9 @@ class Retry(object):
             must be greater than 0.
         maximum (float): The maximum amout of time to delay in seconds.
         multiplier (float): The multiplier applied to the delay.
-        deadline (float): How long to keep retrying in seconds.
-        strict_deadline (bool): If :data:`True`, the last retry will run at
-            ``deadline``, shortening the last sleep interval as necessary.
-            Defaults to :data:`False`.
+        deadline (float): How long to keep retrying in seconds. The last sleep
+            period is shortened as necessary, so that the last retry runs at
+            ``deadline`` (and not considerably beyond it).
     """
 
     def __init__(
@@ -251,15 +247,13 @@ def __init__(
         multiplier=_DEFAULT_DELAY_MULTIPLIER,
         deadline=_DEFAULT_DEADLINE,
         on_error=None,
-        strict_deadline=False,
     ):
         self._predicate = predicate
         self._initial = initial
         self._multiplier = multiplier
         self._maximum = maximum
         self._deadline = deadline
         self._on_error = on_error
-        self._strict_deadline = strict_deadline
 
     def __call__(self, func, on_error=None):
         """Wrap a callable with retry behavior.
@@ -290,19 +284,15 @@ def retry_wrapped_func(*args, **kwargs):
                 sleep_generator,
                 self._deadline,
                 on_error=on_error,
-                strict_deadline=self._strict_deadline
             )
 
         return retry_wrapped_func
 
-    def with_deadline(self, deadline, strict_deadline=False):
+    def with_deadline(self, deadline):
         """Return a copy of this retry with the given deadline.
 
         Args:
             deadline (float): How long to keep retrying.
-            strict_deadline (bool): If :data:`True`, the last retry will run at
-                ``deadline``, shortening the last sleep interval as necessary.
-                Defaults to :data:`False`.
 
         Returns:
             Retry: A new retry instance with the given deadline.
@@ -314,7 +304,6 @@ def with_deadline(self, deadline, strict_deadline=False):
             multiplier=self._multiplier,
             deadline=deadline,
             on_error=self._on_error,
-            strict_deadline=strict_deadline,
         )
 
     def with_predicate(self, predicate):
@@ -334,7 +323,6 @@ def with_predicate(self, predicate):
             multiplier=self._multiplier,
             deadline=self._deadline,
             on_error=self._on_error,
-            strict_deadline=self._strict_deadline,
         )
 
     def with_delay(self, initial=None, maximum=None, multiplier=None):
@@ -356,20 +344,17 @@ def with_delay(self, initial=None, maximum=None, multiplier=None):
             multiplier=multiplier if maximum is not None else self._multiplier,
             deadline=self._deadline,
             on_error=self._on_error,
-            strict_deadline=self._strict_deadline,
         )
 
     def __str__(self):
         return (
             "<Retry predicate={}, initial={:.1f}, maximum={:.1f}, "
-            "multiplier={:.1f}, deadline={:.1f}, on_error={}, "
-            "strict_deadline={}>".format(
+            "multiplier={:.1f}, deadline={:.1f}, on_error={}>".format(
                 self._predicate,
                 self._initial,
                 self._maximum,
                 self._multiplier,
                 self._deadline,
                 self._on_error,
-                self._strict_deadline,
             )
         )

api_core/tests/unit/test_retry.py

@@ -162,7 +162,6 @@ def test_constructor_defaults(self):
         assert retry_._multiplier == 2
         assert retry_._deadline == 120
         assert retry_._on_error is None
-        assert retry_._strict_deadline is False
 
     def test_constructor_options(self):
         _some_function = mock.Mock()
@@ -174,15 +173,13 @@ def test_constructor_options(self):
             multiplier=3,
             deadline=4,
             on_error=_some_function,
-            strict_deadline=True,
         )
         assert retry_._predicate == mock.sentinel.predicate
         assert retry_._initial == 1
         assert retry_._maximum == 2
         assert retry_._multiplier == 3
         assert retry_._deadline == 4
         assert retry_._on_error is _some_function
-        assert retry_._strict_deadline is True
 
     def test_with_deadline(self):
         retry_ = retry.Retry(
@@ -192,12 +189,10 @@ def test_with_deadline(self):
             multiplier=3,
             deadline=4,
             on_error=mock.sentinel.on_error,
-            strict_deadline=True,
         )
-        new_retry = retry_.with_deadline(42, strict_deadline=True)
+        new_retry = retry_.with_deadline(42)
         assert retry_ is not new_retry
         assert new_retry._deadline == 42
-        assert new_retry._strict_deadline is True
 
         # the rest of the attributes should remain the same
         assert new_retry._predicate is retry_._predicate
@@ -214,15 +209,13 @@ def test_with_predicate(self):
             multiplier=3,
             deadline=4,
             on_error=mock.sentinel.on_error,
-            strict_deadline=True,
         )
         new_retry = retry_.with_predicate(mock.sentinel.predicate)
         assert retry_ is not new_retry
         assert new_retry._predicate == mock.sentinel.predicate
 
         # the rest of the attributes should remain the same
         assert new_retry._deadline == retry_._deadline
-        assert new_retry._strict_deadline == retry_._strict_deadline
         assert new_retry._initial == retry_._initial
         assert new_retry._maximum == retry_._maximum
         assert new_retry._multiplier == retry_._multiplier
@@ -236,7 +229,6 @@ def test_with_delay_noop(self):
             multiplier=3,
             deadline=4,
             on_error=mock.sentinel.on_error,
-            strict_deadline=True,
         )
         new_retry = retry_.with_delay()
         assert retry_ is not new_retry
@@ -252,7 +244,6 @@ def test_with_delay(self):
             multiplier=3,
             deadline=4,
             on_error=mock.sentinel.on_error,
-            strict_deadline=True,
         )
         new_retry = retry_.with_delay(initial=1, maximum=2, multiplier=3)
         assert retry_ is not new_retry
@@ -262,7 +253,6 @@ def test_with_delay(self):
 
         # the rest of the attributes should remain the same
         assert new_retry._deadline == retry_._deadline
-        assert new_retry._strict_deadline == retry_._strict_deadline
         assert new_retry._predicate is retry_._predicate
         assert new_retry._on_error is retry_._on_error
 
@@ -279,13 +269,12 @@ def if_exception_type(exc):
             multiplier=2.0,
             deadline=120.0,
             on_error=None,
-            strict_deadline=False,
         )
         assert re.match(
             (
                 r"<Retry predicate=<function.*?if_exception_type.*?>, "
                 r"initial=1.0, maximum=60.0, multiplier=2.0, deadline=120.0, "
-                r"on_error=None, strict_deadline=False>"
+                r"on_error=None>"
             ),
             str(retry_),
         )
@@ -332,7 +321,7 @@ def test___call___and_execute_retry(self, sleep, uniform):
     # Make uniform return half of its maximum, which is the calculated sleep time.
     @mock.patch("random.uniform", autospec=True, side_effect=lambda m, n: n / 2.0)
     @mock.patch("time.sleep", autospec=True)
-    def test___call___and_execute_retry_strict_deadline(self, sleep, uniform):
+    def test___call___and_execute_retry_hitting_deadline(self, sleep, uniform):
 
         on_error = mock.Mock(spec=["__call__"], side_effect=[None] * 10)
         retry_ = retry.Retry(
@@ -341,7 +330,6 @@ def test___call___and_execute_retry_strict_deadline(self, sleep, uniform):
             maximum=1024.0,
             multiplier=2.0,
             deadline=9.9,
-            strict_deadline=True,
         )
 
         utcnow = datetime.datetime.utcnow()
@@ -376,7 +364,7 @@ def increase_time(sleep_delay):
         total_wait = sum(call_args.args[0] for call_args in sleep.call_args_list)
 
         assert last_wait == 2.9   # and not 8.0, because the last delay was shortened
-        assert total_wait == 9.9  # the same as the (strict) deadline
+        assert total_wait == 9.9  # the same as the deadline
 
     @mock.patch("time.sleep", autospec=True)
     def test___init___without_retry_executed(self, sleep):