Open yoshizow opened 1 month ago
Thanks for reporting this issue! I had a quick look and this appears to be an instance where we should at least be clearer, and possibly implement the change we suggest. We'll look into this.
Notes as we investigate:
gapic_v1.method.wrap_method
here. google/api_core/gapic_v1/method.py:_GapicCallable.wrap_method
.default_timeout
parameter appears to become the timeout
parameter in _GapicCallable.__init__()
.
Hello,
I'm using the python-pubsub library and noticed that the timeout behavior is strange. After some investigation, I believe it is due to an incompatibility introduced by https://github.com/googleapis/python-api-core/pull/462.
The default retry and timeout settings in the python-pubsub library are as follows: https://github.com/googleapis/python-pubsub/blob/v2.21.1/google/pubsub_v1/services/publisher/transports/base.py#L166-L181
Before #462 was introduced,
default_timeout=60.0
was interpreted asdefault_timeout=ConstantTimeout(60)
. This should be intended to set the RPC Timeout, i.e., the timeout for a single RPC call not including retries, to 60 seconds. However, after the introduction of #462, this line is now interpreted asdefault_timeout=TimeToDeadlineTimeout(60)
. Since TimeToDeadlineTimeout determines the total timeout time across retries rather than determining the RPC Timeout, I don't believe that it can be used as a direct replacement for ConstantTimeout.The strange behavior I mentioned at the beginning came from this: when the server did not respond in the first 60 seconds, the remaining timeout for RPC call was 0 seconds, and the gRPC client returned a 504 Deadline Exceeded on every retry.
TimeToDeadlineTimeout by itself is useful, but given the original intent, I think it is necessary to specify the RPC Timeout as well as the total timeout.
Finally, in my project, I implemented the following ClampMaxTimeout so that the RPC Timeout can be set while respecting the behavior of TimeToDeadlineTimeout.
To keep backward compatibility, I believe it is desirable to introduce a similar process in python-api-core.