René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Major refactoring of Timeout logic introduced incompatibility about RPC Timeout","articleBody":"Hello,\r\n\r\nI'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.\r\n\r\nThe default retry and timeout settings in the python-pubsub library are as follows:\r\nhttps://github.com/googleapis/python-pubsub/blob/v2.21.1/google/pubsub_v1/services/publisher/transports/base.py#L166-L181\r\n```python\r\ndefault_retry=retries.Retry(\r\n    initial=0.1,\r\n    maximum=60.0,\r\n    multiplier=4,\r\n    predicate=retries.if_exception_type(\r\n        core_exceptions.Aborted,\r\n        core_exceptions.Cancelled,\r\n        core_exceptions.DeadlineExceeded,\r\n        core_exceptions.InternalServerError,\r\n        core_exceptions.ResourceExhausted,\r\n        core_exceptions.ServiceUnavailable,\r\n        core_exceptions.Unknown,\r\n    ),\r\n    deadline=600.0,\r\n),\r\ndefault_timeout=60.0,\r\n```\r\n\r\nBefore #462 was introduced, `default_timeout=60.0` was interpreted as `default_timeout=ConstantTimeout(60)`.\r\nThis should be intended to set the RPC Timeout, i.e., the timeout for a single RPC call not including retries, to 60 seconds.\r\nHowever, after the introduction of #462, this line is now interpreted as `default_timeout=TimeToDeadlineTimeout(60)`.\r\nSince 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.\r\n\r\nThe 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.\r\n\r\nTimeToDeadlineTimeout 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.\r\n\r\nFinally, in my project, I implemented the following ClampMaxTimeout so that the RPC Timeout can be set while respecting the behavior of TimeToDeadlineTimeout.\r\n\r\n```python\r\nRPC_TIMEOUT_SECS   = 60.0\r\nTOTAL_TIMEOUT_SECS = 600.0\r\n...\r\npublisher_options=pubsub_v1.types.PublisherOptions(\r\n    timeout=compose(\r\n        api_core.timeout.TimeToDeadlineTimeout(TOTAL_TIMEOUT_SECS),\r\n        ClampMaxTimeout(RPC_TIMEOUT_SECS),\r\n    ),\r\n    ...\r\n...\r\nclass ClampMaxTimeout(object):\r\n    def __init__(self, max_timeout: float):\r\n        self._max_timeout = max_timeout\r\n\r\n    def __call__(self, func: Callable) -\u003e Callable:\r\n        @functools.wraps(func)\r\n        def func_with_max_timeout(*args, **kwargs):\r\n            if kwargs.get(\"timeout\") is not None:\r\n                kwargs[\"timeout\"] = min(kwargs[\"timeout\"], self._max_timeout)\r\n            else:\r\n                kwargs[\"timeout\"] = self._max_timeout\r\n            return func(*args, **kwargs)\r\n\r\n        return func_with_max_timeout\r\n\r\n    def __str__(self) -\u003e str:\r\n        return \"\u003cClampMaxTimeout max_timeout={:.1f}\u003e\".format(self._max_timeout)\r\n\r\ndef compose(f: Callable, g: Callable) -\u003e Callable:\r\n    return lambda x: f(g(x))\r\n```\r\n\r\nTo keep backward compatibility, I believe it is desirable to introduce a similar process in python-api-core.\r\n","author":{"url":"https://github.com/yoshizow","@type":"Person","name":"yoshizow"},"datePublished":"2024-05-10T06:52:13.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":3},"url":"https://github.com/654/python-api-core/issues/654"}