| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
| return message | ||
|
|
||
|
|
||
| def format_http_response_error(response, method, url, payload=None): |
There was a problem hiding this comment.
nit: add type hints
nit: add code comments to clarify the reason that we're moving away from from_http_response
Sorry, something went wrong.
There was a problem hiding this comment.
addressed with a comment.
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good. Just minor comments/questions. Please address @parthea's comments as well.
Sorry, something went wrong.
| default_timeout=None, | ||
| default_compression=None, | ||
| client_info=client_info.DEFAULT_CLIENT_INFO, | ||
| kind="grpc", |
There was a problem hiding this comment.
OK, and when we call wrap_method from the GAPIC, it will specify this parameter, right? So that will be in a follow-up PR in the generator repo?
Sorry, something went wrong.
There was a problem hiding this comment.
That's correct! Note to myself to update the default value to grpc_asyncio.
Sorry, something went wrong.
There was a problem hiding this comment.
addressed.
Sorry, something went wrong.
There was a problem hiding this comment.
Update the PR description with the right default value.
Sorry, something went wrong.
|
Adding a do not merge label until open comments are addressed and the default value is updated to grpc_asyncio. |
Sorry, something went wrong.
…oogleapis/python-api-core into add-support-for-mapping-rest-callables
…oogleapis/python-api-core into add-support-for-mapping-rest-callables
|
Would this fix the issue in googleapis/gapic-generator-python#1764? Or is this something else? |
Sorry, something went wrong.
|
This PR removes grpc code path from the stack trace when an error is raised from an asynchronous rest call. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This PR implements a _RestCall class to map GoogleAPICallError to an asynchronous rest callable that raises google.auth.exceptions.TransportError.This wrapping is done via a helper method rest_helpers_async.wrap_errors.
A new parameter kind is introduced in wrap_method which can be one of grpc_asyncio (Default Value) or rest_asyncio to distinguish between the type of callables so that we can apply the appropriate wrapping logic.
The property kind is exposed at the transport level here and will be configured when wrapping methods during the transport initialization here.
As a follow up, a similar logic must be implemented for synchronous rest callables to avoid gRPC specific stack trace within a rest call.
RE: Updated Workflow:
I've removed the proposed _RestCall class after determining that it is not needed to wrap a rest callable and just adds unnecessary logic.
The new proposed workflow is as follows:
Essentially, this does the trick if we follow a similar pattern for error handling to what we're doing in a sync rest call.
We have been lazily creating and raising a GoogleAPICallError from a requests.Response within our GAPICs using the from_http_response method here. Therefore, we don't need to add the error handling logic to the callable beforehand (we're currently doing both the things for a sync rest call and should skip wrapping the callable beforehand there too).
This PR only handles the error handling logic for an async REST call. It also introduces a new helper i.e. format_http_response_error to create and raise a GoogleAPICallError for a more generic response since the existing from_http_response is specific to handling a requests response.
Alternate approach for the introduced helper:
The proposed implementation of format_http_response_error takes in payload as an argument so that we don't need to await for it within the call and keep this function synchronous.
Alternatively, we can have an async version of from_http_response that can be used to create GoogleAPICallError for async rest calls.
Follow Up:
As a follow up, we should skip wrapping sync rest callable with grpc_helpers.wrap_errors (similar to what we're doing here) to avoid the unnecessary gRPC specific logic within a rest call stracktrace.