From 2fc2b35f26057650b4bf3c5898ea5a187a3da633 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 22 Aug 2024 15:49:58 +0000 Subject: [PATCH 1/8] feat: add suport for mapping exceptions to rest callables --- google/api_core/gapic_v1/method_async.py | 8 +++- google/api_core/rest_helpers_async.py | 54 ++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 google/api_core/rest_helpers_async.py diff --git a/google/api_core/gapic_v1/method_async.py b/google/api_core/gapic_v1/method_async.py index 24880756..7c6b6d21 100644 --- a/google/api_core/gapic_v1/method_async.py +++ b/google/api_core/gapic_v1/method_async.py @@ -19,7 +19,7 @@ import functools -from google.api_core import grpc_helpers_async +from google.api_core import grpc_helpers_async, rest_helpers_async from google.api_core.gapic_v1 import client_info from google.api_core.gapic_v1.method import _GapicCallable from google.api_core.gapic_v1.method import DEFAULT # noqa: F401 @@ -32,6 +32,7 @@ def wrap_method( default_timeout=None, default_compression=None, client_info=client_info.DEFAULT_CLIENT_INFO, + kind="grpc" ): """Wrap an async RPC method with common behavior. @@ -40,7 +41,10 @@ def wrap_method( and ``compression`` arguments and applies the common error mapping, retry, timeout, metadata, and compression behavior to the low-level RPC method. """ - func = grpc_helpers_async.wrap_errors(func) + if kind == "rest": + func = rest_helpers_async.wrap_errors(func) + else: + func = grpc_helpers_async.wrap_errors(func) metadata = [client_info.to_grpc_metadata()] if client_info is not None else None diff --git a/google/api_core/rest_helpers_async.py b/google/api_core/rest_helpers_async.py new file mode 100644 index 00000000..6bb1ea02 --- /dev/null +++ b/google/api_core/rest_helpers_async.py @@ -0,0 +1,54 @@ +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +"""AsyncIO helpers for :mod:`rest` supporting 3.7+. +""" + + +import functools + +import google.auth +from google.api_core import exceptions + + + +class _RestCall(): + """Wrapped Rest Call to map exceptions.""" + + def __init__(self): + self._call = None + + def with_call(self, call): + """Supplies the call object separately to keep __init__ clean.""" + self._call = call + return self + + async def __call__(self): + try: + return await self._call + except google.auth.exceptions.TransportError as rpc_exc: + raise exceptions.GoogleAPICallError(str(rpc_exc), errors=(rpc_exc,), response=rpc_exc) + + +def wrap_errors(callable_): + """Map errors for REST async callables.""" + + @functools.wraps(callable_) + async def error_remapped_callable(*args, **kwargs): + call = callable_(*args, **kwargs) + call = _RestCall().with_call(call) + response = await call() + return response + + return error_remapped_callable From 573413ac69de1de4223c9fe589a36bda1f84f419 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Fri, 23 Aug 2024 05:55:30 +0000 Subject: [PATCH 2/8] avoid wrapping errors for rest callable --- google/api_core/exceptions.py | 48 +++++++++++++++------ google/api_core/gapic_v1/method_async.py | 6 +-- google/api_core/rest_helpers_async.py | 54 ------------------------ 3 files changed, 36 insertions(+), 72 deletions(-) delete mode 100644 google/api_core/rest_helpers_async.py diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 74f46ef3..5794b9fb 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -475,23 +475,30 @@ def from_http_status(status_code, message, **kwargs): return error +def _format_error_message(error, method, url): + method = method.upper() + message = "{method} {url}: {error}".format( + method=method, + url=url, + error=error, + ) + return message -def from_http_response(response): - """Create a :class:`GoogleAPICallError` from a :class:`requests.Response`. +def format_http_response_error(response, method, url, payload=None): + """Create a :class:`GoogleAPICallError` from a google auth rest response. Args: - response (requests.Response): The HTTP response. + response Union[google.auth.transport.Response, google.auth.aio.transport.Response]: The HTTP response. + method Optional(str): The HTTP request method. + url Optional(str): The HTTP request url. + payload Optional(str): The HTTP response payload. If not passed in, it is read from response for a response type of google.auth.transport.Response. Returns: GoogleAPICallError: An instance of the appropriate subclass of :class:`GoogleAPICallError`, with the message and errors populated from the response. """ - try: - payload = response.json() - except ValueError: - payload = {"error": {"message": response.text or "unknown error"}} - + payload = {} if not payload else payload error_message = payload.get("error", {}).get("message", "unknown error") errors = payload.get("error", {}).get("errors", ()) # In JSON, details are already formatted in developer-friendly way. @@ -504,12 +511,7 @@ def from_http_response(response): ) ) error_info = error_info[0] if error_info else None - - message = "{method} {url}: {error}".format( - method=response.request.method, - url=response.request.url, - error=error_message, - ) + message = _format_error_message(error_message, method, url) exception = from_http_status( response.status_code, @@ -522,6 +524,24 @@ def from_http_response(response): return exception +def from_http_response(response): + """Create a :class:`GoogleAPICallError` from a :class:`requests.Response`. + + Args: + response (requests.Response): The HTTP response. + + Returns: + GoogleAPICallError: An instance of the appropriate subclass of + :class:`GoogleAPICallError`, with the message and errors populated + from the response. + """ + try: + payload = response.json() + except ValueError: + payload = {"error": {"message": response.text or "unknown error"}} + return format_http_response_error(response, response.request.method, response.request.url, payload) + + def exception_class_for_grpc_status(status_code): """Return the exception class for a specific :class:`grpc.StatusCode`. diff --git a/google/api_core/gapic_v1/method_async.py b/google/api_core/gapic_v1/method_async.py index 7c6b6d21..c970e8db 100644 --- a/google/api_core/gapic_v1/method_async.py +++ b/google/api_core/gapic_v1/method_async.py @@ -19,7 +19,7 @@ import functools -from google.api_core import grpc_helpers_async, rest_helpers_async +from google.api_core import grpc_helpers_async from google.api_core.gapic_v1 import client_info from google.api_core.gapic_v1.method import _GapicCallable from google.api_core.gapic_v1.method import DEFAULT # noqa: F401 @@ -41,9 +41,7 @@ def wrap_method( and ``compression`` arguments and applies the common error mapping, retry, timeout, metadata, and compression behavior to the low-level RPC method. """ - if kind == "rest": - func = rest_helpers_async.wrap_errors(func) - else: + if kind == "grpc": func = grpc_helpers_async.wrap_errors(func) metadata = [client_info.to_grpc_metadata()] if client_info is not None else None diff --git a/google/api_core/rest_helpers_async.py b/google/api_core/rest_helpers_async.py deleted file mode 100644 index 6bb1ea02..00000000 --- a/google/api_core/rest_helpers_async.py +++ /dev/null @@ -1,54 +0,0 @@ -# Copyright 2024 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# 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. - -"""AsyncIO helpers for :mod:`rest` supporting 3.7+. -""" - - -import functools - -import google.auth -from google.api_core import exceptions - - - -class _RestCall(): - """Wrapped Rest Call to map exceptions.""" - - def __init__(self): - self._call = None - - def with_call(self, call): - """Supplies the call object separately to keep __init__ clean.""" - self._call = call - return self - - async def __call__(self): - try: - return await self._call - except google.auth.exceptions.TransportError as rpc_exc: - raise exceptions.GoogleAPICallError(str(rpc_exc), errors=(rpc_exc,), response=rpc_exc) - - -def wrap_errors(callable_): - """Map errors for REST async callables.""" - - @functools.wraps(callable_) - async def error_remapped_callable(*args, **kwargs): - call = callable_(*args, **kwargs) - call = _RestCall().with_call(call) - response = await call() - return response - - return error_remapped_callable From 9c76b5ff10243f8c3ad64a86469b76f351deafe8 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Fri, 23 Aug 2024 06:47:06 +0000 Subject: [PATCH 3/8] fix lint issues --- google/api_core/exceptions.py | 6 +++++- google/api_core/gapic_v1/method_async.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 5794b9fb..3836d4a7 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -475,6 +475,7 @@ def from_http_status(status_code, message, **kwargs): return error + def _format_error_message(error, method, url): method = method.upper() message = "{method} {url}: {error}".format( @@ -484,6 +485,7 @@ def _format_error_message(error, method, url): ) return message + def format_http_response_error(response, method, url, payload=None): """Create a :class:`GoogleAPICallError` from a google auth rest response. @@ -539,7 +541,9 @@ def from_http_response(response): payload = response.json() except ValueError: payload = {"error": {"message": response.text or "unknown error"}} - return format_http_response_error(response, response.request.method, response.request.url, payload) + return format_http_response_error( + response, response.request.method, response.request.url, payload + ) def exception_class_for_grpc_status(status_code): diff --git a/google/api_core/gapic_v1/method_async.py b/google/api_core/gapic_v1/method_async.py index c970e8db..fdaed391 100644 --- a/google/api_core/gapic_v1/method_async.py +++ b/google/api_core/gapic_v1/method_async.py @@ -32,7 +32,7 @@ def wrap_method( default_timeout=None, default_compression=None, client_info=client_info.DEFAULT_CLIENT_INFO, - kind="grpc" + kind="grpc", ): """Wrap an async RPC method with common behavior. From 2373149e1037f0ea7a4107d45ef88fc4004c4c55 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Fri, 23 Aug 2024 07:07:39 +0000 Subject: [PATCH 4/8] add test coverage --- tests/asyncio/gapic/test_method_async.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/asyncio/gapic/test_method_async.py b/tests/asyncio/gapic/test_method_async.py index ee206979..f64157b4 100644 --- a/tests/asyncio/gapic/test_method_async.py +++ b/tests/asyncio/gapic/test_method_async.py @@ -252,3 +252,14 @@ async def test_wrap_method_with_overriding_timeout_as_a_number(): assert result == 42 method.assert_called_once_with(timeout=22, metadata=mock.ANY) + + +@pytest.mark.asyncio +async def test_wrap_method_without_wrap_errors(): + fake_call = mock.AsyncMock() + + wrapped_method = gapic_v1.method_async.wrap_method(fake_call, kind="rest") + with mock.patch("google.api_core.grpc_helpers_async.wrap_errors") as method: + await wrapped_method() + + method.assert_not_called() From dd7b5d1110946fa54f223487c5f4ff78a8fa6998 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 9 Sep 2024 19:44:47 +0000 Subject: [PATCH 5/8] address PR comments --- google/api_core/exceptions.py | 7 ++++--- google/api_core/gapic_v1/method_async.py | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 3836d4a7..97ac6b65 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -476,7 +476,7 @@ def from_http_status(status_code, message, **kwargs): return error -def _format_error_message(error, method, url): +def _format_rest_error_message(error, method, url): method = method.upper() message = "{method} {url}: {error}".format( method=method, @@ -486,7 +486,8 @@ def _format_error_message(error, method, url): return message -def format_http_response_error(response, method, url, payload=None): +# TODO(https://github.com/googleapis/python-api-core/issues/691): Add type hint for response. +def format_http_response_error(response, method: str, url: str, payload: str = None): """Create a :class:`GoogleAPICallError` from a google auth rest response. Args: @@ -513,7 +514,7 @@ def format_http_response_error(response, method, url, payload=None): ) ) error_info = error_info[0] if error_info else None - message = _format_error_message(error_message, method, url) + message = _format_rest_error_message(error_message, method, url) exception = from_http_status( response.status_code, diff --git a/google/api_core/gapic_v1/method_async.py b/google/api_core/gapic_v1/method_async.py index fdaed391..fbba4aa5 100644 --- a/google/api_core/gapic_v1/method_async.py +++ b/google/api_core/gapic_v1/method_async.py @@ -25,6 +25,7 @@ from google.api_core.gapic_v1.method import DEFAULT # noqa: F401 from google.api_core.gapic_v1.method import USE_DEFAULT_METADATA # noqa: F401 +_DEFAULT_ASYNC_TRANSPORT_KIND = "grpc_asyncio" def wrap_method( func, @@ -32,7 +33,7 @@ def wrap_method( default_timeout=None, default_compression=None, client_info=client_info.DEFAULT_CLIENT_INFO, - kind="grpc", + kind=_DEFAULT_ASYNC_TRANSPORT_KIND, ): """Wrap an async RPC method with common behavior. @@ -41,7 +42,7 @@ def wrap_method( and ``compression`` arguments and applies the common error mapping, retry, timeout, metadata, and compression behavior to the low-level RPC method. """ - if kind == "grpc": + if kind == _DEFAULT_ASYNC_TRANSPORT_KIND: func = grpc_helpers_async.wrap_errors(func) metadata = [client_info.to_grpc_metadata()] if client_info is not None else None From f7ebadd0aa6a92b3021d1e24a893be9f42270de1 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Mon, 9 Sep 2024 19:46:51 +0000 Subject: [PATCH 6/8] =?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 --- google/api_core/gapic_v1/method_async.py | 1 + 1 file changed, 1 insertion(+) diff --git a/google/api_core/gapic_v1/method_async.py b/google/api_core/gapic_v1/method_async.py index fbba4aa5..c0f38c0e 100644 --- a/google/api_core/gapic_v1/method_async.py +++ b/google/api_core/gapic_v1/method_async.py @@ -27,6 +27,7 @@ _DEFAULT_ASYNC_TRANSPORT_KIND = "grpc_asyncio" + def wrap_method( func, default_retry=None, From be83346bf596ff3cd39e375c4940185c9f2deeec Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 9 Sep 2024 19:57:18 +0000 Subject: [PATCH 7/8] fix lint issues --- google/api_core/exceptions.py | 11 ++++++++--- google/api_core/gapic_v1/method_async.py | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index 97ac6b65..ec206d80 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -22,7 +22,7 @@ from __future__ import unicode_literals import http.client -from typing import Dict +from typing import Optional, Dict from typing import Union import warnings @@ -486,15 +486,20 @@ def _format_rest_error_message(error, method, url): return message +# NOTE: We're moving away from `from_http_status` because it expects an aiohttp response compared +# to `format_http_response_error` which expects a more abstract response from google.auth and is +# compatible with both sync and async response types. # TODO(https://github.com/googleapis/python-api-core/issues/691): Add type hint for response. -def format_http_response_error(response, method: str, url: str, payload: str = None): +def format_http_response_error( + response, method: str, url: str, payload: Optional[Dict] = None +): """Create a :class:`GoogleAPICallError` from a google auth rest response. Args: response Union[google.auth.transport.Response, google.auth.aio.transport.Response]: The HTTP response. method Optional(str): The HTTP request method. url Optional(str): The HTTP request url. - payload Optional(str): The HTTP response payload. If not passed in, it is read from response for a response type of google.auth.transport.Response. + payload Optional(dict): The HTTP response payload. If not passed in, it is read from response for a response type of google.auth.transport.Response. Returns: GoogleAPICallError: An instance of the appropriate subclass of diff --git a/google/api_core/gapic_v1/method_async.py b/google/api_core/gapic_v1/method_async.py index fbba4aa5..c0f38c0e 100644 --- a/google/api_core/gapic_v1/method_async.py +++ b/google/api_core/gapic_v1/method_async.py @@ -27,6 +27,7 @@ _DEFAULT_ASYNC_TRANSPORT_KIND = "grpc_asyncio" + def wrap_method( func, default_retry=None, From 324a2a010162ec73031046d631b0fc99f14c253d Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 10 Sep 2024 01:36:35 +0000 Subject: [PATCH 8/8] fix for none type method --- google/api_core/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/api_core/exceptions.py b/google/api_core/exceptions.py index ec206d80..8dc8c08f 100644 --- a/google/api_core/exceptions.py +++ b/google/api_core/exceptions.py @@ -477,7 +477,7 @@ def from_http_status(status_code, message, **kwargs): def _format_rest_error_message(error, method, url): - method = method.upper() + method = method.upper() if method else None message = "{method} {url}: {error}".format( method=method, url=url,