René's URL Explorer Experiment

Title: fix: Major refactoring of Polling, Retry and Timeout logic in v1 branch by parthea · Pull Request #474 · googleapis/python-api-core · GitHub

Open Graph Title: fix: Major refactoring of Polling, Retry and Timeout logic in v1 branch by parthea · Pull Request #474 · googleapis/python-api-core

X Title: fix: Major refactoring of Polling, Retry and Timeout logic in v1 branch by parthea · Pull Request #474 · googleapis/python-api-core

Description: BEGIN_COMMIT_OVERRIDE feat: Allow representing enums with their unqualified symbolic names in headers in v1 branch (#462) fix: Major refactoring of Polling, Retry and Timeout logic in v1 branch (#465) END_COMMIT_OVERRIDE Including notes from PR #462 This is in response to https://freeman.vc/notes/aws-vs-gcp-reliability-is-wildly-different, which triggered an investigation of the whole Polling/Retry/Timeout behavior in Python GAPIC clients and revealed many fundamental >flaws in its implementation. To properly describe the refactoring in this PR we need to stick to a rigorous terminology, as vague definitions of retries, timeouts, polling and related concepts seems to be the main source of the present bugs and overall >confusion among both groups: users of the library and creators of the library. Please check the updated (in this PR) documentation of the google.api_core.retry.Retry class and the google.api_core.future.polling.Polling.result>() method for the proper definitions and context. Note, the overall semantics around Polling, Retry and Timeout remains quite confusing even after refactoring (although it is now more or less rigorously defined), but it was as clean as I could make it while still maintaining >backward compatibility of the whole library. The quick summary of the changes in this PR: Properly define and fix the application of Deadline and Timeout concepts. Please check the updated documentation for the google.api_core.retry.Retry class for the actual definitions. Originally the deadline has been used >to represent timeouts conflating the two concepts. As result this PR replaces deadline arguments with timeout ones in as backward-compatible manner as possible (i.e. backward compatible in all practical applications). Properly define RPC Timeout, Retry Timeout and Polling Timeout and how a generic Timeout concept (aka Logical Timeout) is mapped to one of those depending on the context. Please check google.api_core.retry.Retry class >documentation for details. Properly define and fix the application of Retry and Polling concepts. Please check the updated documentation for google.api_core.future.polling.PollingFuture.result() for details. Separate retry and polling configurations for Polling future, as these are two different concepts (although both operating on Retry class). Originally both retry and polling configurations were controlled by a single >retry parameter, merging configuration regarding how "rpc error responses" and how "operation not completed" responses are supposed to be handled. For the following config properties - Retry (including Retry Timeout), Polling (including Polling Timeout) and RPC Timeout - fix and properly define how each of the above properties gets configured and which config >gets precedence in case of a conflict (check PollingFuture.result() method documentation for details). Each of those properties can be specified as follows: directly provided by the user for each call, specified during gapic >generation time from config values in grpc_service_config.json file (for Retry and RPC Timeout) and gapic.yaml file (for Polling Timeout), or be provided as a hard-coded basic default values in python-api-core library >itself. This alo includes fixing the per-call polling config propagation logic (the polling/retry configs supplied to PollingFuture.result() used to be ignored for actual call). Deprecate ExponentialTimeout, ConstantTimeout and related logic as those are outdated concepts and are not consistent with the other GAPIC Languages. Replace it with TimeToDeadlineTimeout to be consistent with how the >rest of the languages do it. Deprecate google.api_core.operations_v1.config as it is an outdated concept and self-inconsistent (as all gapic clients provide configuraiton in code). The configs are directly provided in code instead. Switch randomized delay calculation from delay being treated as expected value for randomized_delay to delay being treated as maximum value for randomized_delay (i.e. the new expected valud for randomized_delay is >delay / 2). See the exponential_sleep_generator() method implementation for details. This is needed to make Python implementation of retries and polling exponential backoff consistent with the rest of GAPIC languages. Also >fix the uncontrollable growth of delay value (since it is a subject of exponential growth, the delay value was quickly reaching "infinity" value, and the whole thing was not failing simply due to python being a very >forgiving language which forgives multiplying "infinity" by a number (inf * number = inf) binstead of simply overflowing to a (most likely) negative number). Also essentially rollback the https://github.com/googleapis/>python-api-core/commit/52f12af027568ddeae7fb6b1e3d2111cce1bac08 change, since that is inconsistent with the other languages and damages uniform distibution of retry delays artificially shifting their concentration towards the >end of timeout. Fix url construction in OperationsRestTransport. Without this fix the polling logic for REST transport was completely broken (is not affecting Compute client, as that one has custom LRO). Last but not least: change the default values for Polling logic to be the following: initial=1.0 (same as before), maximum=20.0 (was 60), multiplier=1.5 (was 2.0), timeout=900 (was 120, but due to timeout >resolution logic was actually None (i.e. infinity)). This, in conjunction with changed calculation of randomized delay (i.e. its expected value now being delay / 2) overall makes polling logic much less aggressive in terms of >increasing delays between each polling iteration, making LRO return much earlier for users on average, but still keeping a healthy balance between strain put on both client and server by polling and responsiveness of LROs for >user. *The design doc summarising all the changes and reasons for them is in progress. In addition to the timeout/retry fixes, this PR has some other non-related technical fixes: Fix and improve code coverage (and explicitly disables lines which are not supposed to be covered) Define Python 3.10 as the default python version used in CI. Upgrade to Sphinx 4.2.0, as it is the one supporting Python 3.10

Open Graph Description: BEGIN_COMMIT_OVERRIDE feat: Allow representing enums with their unqualified symbolic names in headers in v1 branch (#462) fix: Major refactoring of Polling, Retry and Timeout logic in v1 branch (#4...

X Description: BEGIN_COMMIT_OVERRIDE feat: Allow representing enums with their unqualified symbolic names in headers in v1 branch (#462) fix: Major refactoring of Polling, Retry and Timeout logic in v1 branch (#4...

Opengraph URL: https://github.com/googleapis/python-api-core/pull/474

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width