feat: Implement python retrying connection, which generically retries stream errors #4
Conversation
| from google.api_core.exceptions import GoogleAPICallError | ||
|
|
||
| retryable_codes = { | ||
| StatusCode.DEADLINE_EXCEEDED, StatusCode.ABORTED, StatusCode.INTERNAL, StatusCode.UNAVAILABLE, StatusCode.UNKNOWN |
There was a problem hiding this comment.
Have you considered adding CANCELLED and/or RESOURCE_EXHAUSTED?
The retry codes for the CPS library are here:
https://github.com/googleapis/python-pubsub/blob/0132a4680e0727ce45d5e27d98ffc9f3541a0962/google/cloud/pubsub_v1/gapic/publisher_client_config.py#L4
There was a problem hiding this comment.
CANCELLED should not be retried, it is a bug if it leaks to the client on a non-client initiated action.
RESOURCE_EXHAUSTED is the same: It should only be returned when the client is actually out of resources.
This is the same set of codes as the java client library.
There was a problem hiding this comment.
Please mind that I'm not familiar with the PubSub Lite library, thus take my review with a grain of salt, but I do think it's fine all in all.
Made two remarks that might be an opportunity for refactoring/improvement, but they should not be considered showstoppers.
* feat: Implement python retrying connection, which generically retries stream errors. * fix: Add asynctest to tests_require. * feat: Implement DefaultRouingPolicy * fix: Add class comments. * feat: Implement python retrying connection, which generically retries stream errors (#4) * feat: Implement python retrying connection, which generically retries stream errors. * fix: Add asynctest to tests_require. * fix: Add class comments. * feat: Implement DefaultRouingPolicy * fix: Add class comments. * docs: Add comment to DefaultRoutingPolicy.
This mirrors the java equivalent at https://github.com/googleapis/java-pubsublite/blob/master/google-cloud-pubsublite/src/main/java/com/google/cloud/pubsublite/internal/wire/RetryingConnectionImpl.java