From 1a64fff3fece89f3fa8b8d5c959d489c08c11509 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Mon, 14 Mar 2022 20:36:23 +0000 Subject: [PATCH 1/7] fix: ensure exception is available when BackgroundConsumer open stream fails --- google/api_core/bidi.py | 6 +++++- tests/unit/test_bidi.py | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/google/api_core/bidi.py b/google/api_core/bidi.py index 4b4963f7..12cfc8ee 100644 --- a/google/api_core/bidi.py +++ b/google/api_core/bidi.py @@ -276,7 +276,11 @@ def open(self): request_generator = _RequestQueueGenerator( self._request_queue, initial_request=self._initial_request ) - call = self._start_rpc(iter(request_generator), metadata=self._rpc_metadata) + try: + call = self._start_rpc(iter(request_generator), metadata=self._rpc_metadata) + except exceptions.GoogleAPICallError as exc: + self._on_call_done(exc) + raise request_generator.call = call diff --git a/tests/unit/test_bidi.py b/tests/unit/test_bidi.py index 7fb16209..a6c4f00b 100644 --- a/tests/unit/test_bidi.py +++ b/tests/unit/test_bidi.py @@ -804,6 +804,19 @@ def test_wake_on_error(self): while consumer.is_active: pass + def test_rpc_callback_fires_when_consumer_start_fails(self): + expected_exception = exceptions.InvalidArgument("test") + callback = mock.Mock(spec=["__call__"]) + + rpc, _ = make_rpc() + bidi_rpc = bidi.BidiRpc(rpc) + bidi_rpc.add_done_callback(callback) + bidi_rpc._start_rpc.side_effect = expected_exception + + consumer = bidi.BackgroundConsumer(bidi_rpc, on_response=lambda: None) + consumer.start() + assert callback.call_args.args[0] == expected_exception + def test_consumer_expected_error(self, caplog): caplog.set_level(logging.DEBUG) From 97c236391b50ef4b2f3bad8221f4f117efeaf6aa Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 15 Mar 2022 13:05:40 +0000 Subject: [PATCH 2/7] chore: fix coverage --- tests/unit/test_bidi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_bidi.py b/tests/unit/test_bidi.py index a6c4f00b..5c11d6a9 100644 --- a/tests/unit/test_bidi.py +++ b/tests/unit/test_bidi.py @@ -813,7 +813,7 @@ def test_rpc_callback_fires_when_consumer_start_fails(self): bidi_rpc.add_done_callback(callback) bidi_rpc._start_rpc.side_effect = expected_exception - consumer = bidi.BackgroundConsumer(bidi_rpc, on_response=lambda: None) + consumer = bidi.BackgroundConsumer(bidi_rpc, on_response=None) consumer.start() assert callback.call_args.args[0] == expected_exception From c7f63bd270d81f75abdf0282bd3c4be4de02bdbd Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Fri, 13 Oct 2023 16:59:44 +0000 Subject: [PATCH 3/7] Address review comments --- google/api_core/bidi.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/google/api_core/bidi.py b/google/api_core/bidi.py index 17d9fc9f..0eb4a5dc 100644 --- a/google/api_core/bidi.py +++ b/google/api_core/bidi.py @@ -265,6 +265,9 @@ def add_done_callback(self, callback): self._callbacks.append(callback) def _on_call_done(self, future): + # Note that grpc's "future" here is also a grpc.RpcError. + # `grpc.RpcError` is also `grpc.call` based on + # https://github.com/grpc/grpc/issues/10885#issuecomment-302651331 for callback in self._callbacks: callback(future) From 3304ef10e6a66bc75bfbb182886d1cde7b367421 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Fri, 13 Oct 2023 17:20:48 +0000 Subject: [PATCH 4/7] revert --- google/api_core/bidi.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/google/api_core/bidi.py b/google/api_core/bidi.py index 0eb4a5dc..17d9fc9f 100644 --- a/google/api_core/bidi.py +++ b/google/api_core/bidi.py @@ -265,9 +265,6 @@ def add_done_callback(self, callback): self._callbacks.append(callback) def _on_call_done(self, future): - # Note that grpc's "future" here is also a grpc.RpcError. - # `grpc.RpcError` is also `grpc.call` based on - # https://github.com/grpc/grpc/issues/10885#issuecomment-302651331 for callback in self._callbacks: callback(future) From e120a0cdb589d390848b0711ff21c9ff4aab26a9 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Fri, 13 Oct 2023 17:36:29 +0000 Subject: [PATCH 5/7] address review feedback --- google/api_core/bidi.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/google/api_core/bidi.py b/google/api_core/bidi.py index 17d9fc9f..11a32ffa 100644 --- a/google/api_core/bidi.py +++ b/google/api_core/bidi.py @@ -92,10 +92,7 @@ def _is_active(self): # Note: there is a possibility that this starts *before* the call # property is set. So we have to check if self.call is set before # seeing if it's active. - if self.call is not None and not self.call.is_active(): - return False - else: - return True + return self.call is not None and self.call.is_active() def __iter__(self): if self._initial_request is not None: From 753a591bb7f9603929fc81c6880c58237c23a49c Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Fri, 13 Oct 2023 18:33:30 +0000 Subject: [PATCH 6/7] raise grpc.RpcError instead of GoogleAPICallError --- google/api_core/bidi.py | 8 +++++++- tests/unit/test_bidi.py | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/google/api_core/bidi.py b/google/api_core/bidi.py index 11a32ffa..74abc495 100644 --- a/google/api_core/bidi.py +++ b/google/api_core/bidi.py @@ -262,6 +262,10 @@ def add_done_callback(self, callback): self._callbacks.append(callback) def _on_call_done(self, future): + # This occurs when the RPC errors or is successfully terminated. + # Note that grpc's "future" here can also be a grpc.RpcError. + # See note in https://github.com/grpc/grpc/issues/10885#issuecomment-302651331 + # that `grpc.RpcError` is also `grpc.call`. for callback in self._callbacks: callback(future) @@ -276,7 +280,9 @@ def open(self): try: call = self._start_rpc(iter(request_generator), metadata=self._rpc_metadata) except exceptions.GoogleAPICallError as exc: - self._on_call_done(exc) + # The original `grpc.RpcError` (which is usually also a `grpc.Call`) is + # available from the ``response`` property on the mapped exception. + self._on_call_done(exc.response) raise request_generator.call = call diff --git a/tests/unit/test_bidi.py b/tests/unit/test_bidi.py index ad6f94be..d4d663fc 100644 --- a/tests/unit/test_bidi.py +++ b/tests/unit/test_bidi.py @@ -805,7 +805,7 @@ def test_wake_on_error(self): pass def test_rpc_callback_fires_when_consumer_start_fails(self): - expected_exception = exceptions.InvalidArgument("test") + expected_exception = exceptions.InvalidArgument("test", response=grpc.StatusCode.INVALID_ARGUMENT) callback = mock.Mock(spec=["__call__"]) rpc, _ = make_rpc() @@ -815,7 +815,7 @@ def test_rpc_callback_fires_when_consumer_start_fails(self): consumer = bidi.BackgroundConsumer(bidi_rpc, on_response=None) consumer.start() - assert callback.call_args.args[0] == expected_exception + assert callback.call_args.args[0] == grpc.StatusCode.INVALID_ARGUMENT def test_consumer_expected_error(self, caplog): caplog.set_level(logging.DEBUG) From 3c1daa9dc5357bb276304acec5c17b33b09032fd Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 13 Oct 2023 18:35:17 +0000 Subject: [PATCH 7/7] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- tests/unit/test_bidi.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_bidi.py b/tests/unit/test_bidi.py index d4d663fc..84ac9dc5 100644 --- a/tests/unit/test_bidi.py +++ b/tests/unit/test_bidi.py @@ -805,7 +805,9 @@ def test_wake_on_error(self): pass def test_rpc_callback_fires_when_consumer_start_fails(self): - expected_exception = exceptions.InvalidArgument("test", response=grpc.StatusCode.INVALID_ARGUMENT) + expected_exception = exceptions.InvalidArgument( + "test", response=grpc.StatusCode.INVALID_ARGUMENT + ) callback = mock.Mock(spec=["__call__"]) rpc, _ = make_rpc()