Apply suggestions from code review

comments, typos, etc

Co-authored-by: Victor Chudnovsky <vchudnov@google.com>

google/api_core/retry.py

@@ -415,7 +415,7 @@ def on_error(e):
                     nonlocal stream_idx
                     stream_idx = 0
                 # build retryable
-                retryable_gen = Retry(is_stream=True, on_error=on_error, ...)(target)
+                retryable_gen = Retry(is_stream=True,...)(target)
                 # keep track of what has been yielded out of filter
                 seen_items = []
                 for item in retryable_gen():
@@ -440,7 +440,7 @@ def on_error(e):
             a retryable exception. Any error raised by this function will
             *not* be caught.
         is_stream (bool): Indicates whether the input function
-            should be treated as an stream function (i.e. a Generator,
+            should be treated as a stream function (i.e. a Generator,
             or function that returns an Iterable). If True, the iterable
             will be wrapped with retry logic, and any failed outputs will
             restart the stream. If False, only the input function call itself

google/api_core/retry_async.py

@@ -220,7 +220,7 @@ async def attempt_with_modified_request(target, request, seen_items=[]):
                     yield item
                     seen_items.append(item)
 
-            retry_wrapped = AsyncRetry(is_stream=True)(attempt_with_modified_request, target, request, [])
+            retry_wrapped = AsyncRetry(is_stream=True,...)(attempt_with_modified_request, target, request, [])
 
         2. Wrap the retry generator
             Alternatively, you can wrap the retryable generator itself before
@@ -238,7 +238,7 @@ def on_error(e):
                         nonlocal stream_idx
                         stream_idx = 0
                     # build retryable
-                    retryable_gen = AsyncRetry(is_stream=True, on_error=on_error, ...)(target)
+                    retryable_gen = AsyncRetry(is_stream=True, ...)(target)
                     # keep track of what has been yielded out of filter
                     seen_items = []
                     async for item in retryable_gen:
@@ -263,7 +263,7 @@ def on_error(e):
             a retryable exception. Any error raised by this function will
             *not* be caught.
         is_stream (bool): Indicates whether the input function
-            should be treated as an stream function (i.e. an AsyncGenerator,
+            should be treated as a stream function (i.e. an AsyncGenerator,
             or function or coroutine that returns an AsyncIterable).
             If True, the iterable will be wrapped with retry logic, and any
             failed outputs will restart the stream. If False, only the input

google/api_core/retry_streaming_async.py

@@ -11,6 +11,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+
 """
 Generator wrapper for retryable streaming RPCs.
 This function will be used when initilizing a retry with
@@ -65,7 +66,7 @@ async def retry_target_stream(
     """Create a generator wrapper that retries the wrapped stream if it fails.
 
     This is the lowest-level retry helper. Generally, you'll use the
-    higher-level retry helper :class:`Retry`.
+    higher-level retry helper :class:`AsyncRetry`.
 
     Args:
         target: The generator function to call and retry. This must be a
@@ -91,7 +92,7 @@ async def retry_target_stream(
             on timeout, or the last exception encountered otherwise.
 
     Returns:
-        AssyncGenerator: A retryable generator that wraps the target generator function.
+        AsyncGenerator: A retryable generator that wraps the target generator function.
 
     Raises:
         ValueError: If the sleep generator stops yielding values.

tests/asyncio/test_retry_async.py

@@ -576,7 +576,7 @@ async def _mock_send_gen():
 
         generator = await decorated()
         result = await generator.__anext__()
-        # fist yield should be None
+        # first yield should be None
         assert result is None
         in_messages = ["test_1", "hello", "world"]
         out_messages = []
@@ -600,7 +600,7 @@ async def test___call___generator_send_retry(self, sleep):
         )
         generator = await retry_(self._generator_mock)(error_on=3)
         with pytest.raises(TypeError) as exc_info:
-            await generator.asend("can not send to fresh generator")
+            await generator.asend("cannot send to fresh generator")
             assert exc_info.match("can't send non-None value")
 
         # error thrown on 3
@@ -653,6 +653,8 @@ async def test___call___with_generator_throw(self, sleep):
         """
         Throw should be passed through retry into target generator
         """
+
+        # The generator should not retry when it encounters a non-retryable error
         retry_ = retry_async.AsyncRetry(
             predicate=retry_async.if_exception_type(ValueError),
             is_stream=True,
@@ -668,7 +670,8 @@ async def test___call___with_generator_throw(self, sleep):
         with pytest.raises(StopAsyncIteration):
             # calling next on closed generator should raise error
             await generator.__anext__()
-        # should retry if throw retryable exception
+
+        # In contrast, the generator should retry if we throw a retryable exception
         exception_list = []
         generator = await decorated(10, exceptions_seen=exception_list)
         for i in range(2):
@@ -679,7 +682,7 @@ async def test___call___with_generator_throw(self, sleep):
         # calling next on closed generator should not raise error
         assert await generator.__anext__() == 1
 
-    @pytest.mark.parametrize("awaitale_wrapped", [True, False])
+    @pytest.mark.parametrize("awaitable_wrapped", [True, False])
     @mock.patch("asyncio.sleep", autospec=True)
     @pytest.mark.asyncio
     async def test___call___with_iterable_send(self, sleep, awaitale_wrapped):
@@ -702,11 +705,9 @@ async def __anext__(self):
 
             return CustomIterable()
 
-        if awaitale_wrapped:
-
+        if awaitable_wrapped:
             async def wrapper():
                 return iterable_fn()
-
             decorated = retry_(wrapper)
         else:
             decorated = retry_(iterable_fn)
@@ -718,7 +719,7 @@ async def wrapper():
         await retryable.asend("test2") == 2
         await retryable.asend("test3") == 3
 
-    @pytest.mark.parametrize("awaitale_wrapped", [True, False])
+    @pytest.mark.parametrize("awaitable_wrapped", [True, False])
     @mock.patch("asyncio.sleep", autospec=True)
     @pytest.mark.asyncio
     async def test___call___with_iterable_close(self, sleep, awaitale_wrapped):
@@ -762,7 +763,7 @@ async def wrapper():
         with pytest.raises(StopAsyncIteration):
             await new_retryable.__anext__()
 
-    @pytest.mark.parametrize("awaitale_wrapped", [True, False])
+    @pytest.mark.parametrize("awaitable_wrapped", [True, False])
     @mock.patch("asyncio.sleep", autospec=True)
     @pytest.mark.asyncio
     async def test___call___with_iterable_throw(self, sleep, awaitale_wrapped):

tests/unit/test_retry.py

@@ -721,7 +721,8 @@ def iterable_fn(n):
         retryable.close()
         with pytest.raises(StopIteration):
             next(retryable)
-        # try closing new generator
+
+        # try closing a new generator
         retryable = decorated(10)
         retryable.close()
         with pytest.raises(StopIteration):
@@ -751,7 +752,8 @@ def iterable_fn(n):
             retryable.throw(TypeError)
         with pytest.raises(StopIteration):
             next(retryable)
-        # try throwing with new generator
+
+        # try throwing with a new generator
         retryable = decorated(10)
         with pytest.raises(ValueError):
             retryable.throw(ValueError)