Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: Major refactoring of Polling, Retry and Timeout logic#462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
parthea merged 12 commits intogoogleapis:mainfromvam-google:master
Nov 10, 2022

Conversation

vam-google
Copy link
Contributor

@vam-googlevam-google commentedOct 19, 2022
edited
Loading

If you are a user of python-api-core and experience polling timing out after ~15 minutes (900s) in your python code after this change, please make sure that instead of callingPollingFuture.result() you call it with additonaltimeout argument like this:PollingFuture.result(timeout = <desired timeout in seconds>) orPollingFuture.result(timeout = None) (for infinite timeout, but infinite timeouts are stronglydiscouraged) for your code to stop timing out at 900s.

The core libraries cannot and should not have infitine polling as as default behavior. The fact that it had it like that in python for years (unlike other GCP-supported languages) was a bug, as it contradicts original cross-language LRO methods design in GAPIC libraries and in general users are not supposed to be put into an infinite-loop scenario implicitly. If an LRO can actually ran for hours or even days and it is Ok, then users are expected to acknowledge this fact by providing the huge timeouts explicitly.

This is in response tohttps://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 thegoogle.api_core.retry.Retry class and thegoogle.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:

  1. Properly define and fix the application of Deadline and Timeout concepts. Please check the updated documentation for thegoogle.api_core.retry.Retry class for the actual definitions. Originally thedeadline has been used to represent timeouts conflating the two concepts. As result this PR replacesdeadline arguments withtimeout ones in as backward-compatible manner as possible (i.e. backward compatible in all practical applications).

  2. 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 checkgoogle.api_core.retry.Retry class documentation for details.

  3. Properly define and fix the application of Retry and Polling concepts. Please check the updated documentation forgoogle.api_core.future.polling.PollingFuture.result() for details.

  4. Separateretry andpolling configurations for Polling future, as these are two different concepts (although both operating onRetry class). Originally both retry and polling configurations were controlled by a singleretry parameter, merging configuration regarding how "rpc error responses" and how "operation not completed" responses are supposed to be handled.

  5. For the following config properties -Retry (includingRetry Timeout),Polling (includingPolling Timeout) andRPC Timeout - fix and properly define how each of the above properties gets configured and which config gets precedence in case of a conflict (checkPollingFuture.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 ingrpc_service_config.json file (for Retry and RPC Timeout) andgapic.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 toPollingFuture.result() used to be ignored for actual call).

  6. DeprecateExponentialTimeout,ConstantTimeout and related logic as those are outdated concepts and are not consistent with the other GAPIC Languages. Replace it withTimeToDeadlineTimeout to be consistent with how the rest of the languages do it.

  7. Deprecategoogle.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.

  8. Switch randomized delay calculation fromdelay being treated as expected value for randomized_delay todelay being treated as maximum value forrandomized_delay (i.e. the new expected valud forrandomized_delay isdelay / 2). See theexponential_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 ofdelay value (since it is a subject of exponential growth, thedelay 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 the52f12af 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.

  9. Fix url construction inOperationsRestTransport. Without this fix the polling logic for REST transport was completely broken (is not affecting Compute client, as that one has custom LRO).

  10. Last but not least: change the default values for Polling logic to be the following:initial=1.0 (same as before),maximum=20.0 (was60),multiplier=1.5 (was2.0),timeout=900 (was120, 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 beingdelay / 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)
  • DefinePython 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

This is in response tohttps://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 implementaiton.To properly describe the refactoring this PR does 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 overal confusion among both groups: users of the library and creators of the library. Please check the documentation of the `google.api_core.retry.Retry` class 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 clean as I could make it while still maintaining backward compatibility of the whole library.The quick summary of the changes in this PR:1) 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).2) Properly define RPC Timeout, Retry Timeout and Pollint 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.3) 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.4) 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.5) 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), or be provided as a hard-coded basic default values in python-api-core library itself.6) Fix the per-call polling config propagation logic (the polling/retry configs supplied to `PollingFuture.result()` used to be ignored for actual call).7) Deprecate the usage of `deadline` terminology in the whole library and backward-compatibly replace it with timeout. This is essential as what has been called "deadline" in this library was actually "timeout" as it is defined in `google.api_core.retry.Retry` class documentation.8) 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.9) 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.10) 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).11) 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).12) Las 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.
@vam-googlevam-google requested review froma team ascode ownersOctober 19, 2022 02:43
@product-auto-labelproduct-auto-labelbot added the size: xlPull request size is extra large. labelOct 19, 2022
@vam-googlevam-google changed the titlefix: Major refactoring and fix for Polling, Retry and Timeout logicfix: Major refactoring of Polling, Retry and Timeout logicOct 19, 2022
vam-google added a commit to vam-google/gapic-generator-python that referenced this pull requestOct 27, 2022
Also fix LRO for REST transport. This PR makes generated gapics appeciate timeout values from grpc_service_config.json instead of overriding them with None (which means no timeout)It is basically a direct fix forgoogleapis#1477.This PR depends ongoogleapis/python-api-core#462, and expects `setup.py.j2` templates to be updated aftergoogleapis/python-api-core#462 gets pushed and released with new version.
@vam-google
Copy link
ContributorAuthor

@atulep Addressed your comments PTAL

@vam-google
Copy link
ContributorAuthor

@vam-google test

Copy link
Contributor

@vchudnov-gvchudnov-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I have not looked carefully at the tests yet, but so far this looks great. I will be running some manual tests to complement what Tony and Aza have been doing.

Most of the comments are documentation tweaks, so hopefully you can just accept them on GitHub and pull them into your local repo before making any other changes.

_OperationNotComplete,
exceptions.TooManyRequests,
exceptions.InternalServerError,
exceptions.BadGateway,
exceptions.ServiceUnavailable,
)
DEFAULT_RETRY = retry.Retry(predicate=RETRY_PREDICATE)

# DEPRECATED, use DEFAULT_POLLING to configure LRO polling logic. Construct
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Could you be explicit about how to use DEFAULT_RETRY? It's not immediately clear how that relates to this sentence.
Also, maybe being more explicit about "baseline".

Maybe something like "Construct the Retry object using DEFAULT_RETRY as a baseline and then modifying specific parameters as needed"

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The baseliine part was there before. Just keeping it as is, as the current recommendation is simply to not used that at all for anything.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

As for being explicit on usage of DEFAULT_RETRY, it is deprecated, so it should not be used.

vam-googleand others added3 commitsNovember 8, 2022 09:30
Co-authored-by: Victor Chudnovsky <vchudnov@google.com>
Co-authored-by: Victor Chudnovsky <vchudnov@google.com>
@partheaparthea merged commit434253d intogoogleapis:mainNov 10, 2022
parthea pushed a commit that referenced this pull requestDec 1, 2022
* fix: Major refactoring and fix for Polling, Retry and Timeout logicThis is in response tohttps://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 implementaiton.To properly describe the refactoring this PR does 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 overal confusion among both groups: users of the library and creators of the library. Please check the documentation of the `google.api_core.retry.Retry` class 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 clean as I could make it while still maintaining backward compatibility of the whole library.The quick summary of the changes in this PR:1) 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).2) Properly define RPC Timeout, Retry Timeout and Pollint 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.3) 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.4) 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.5) 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), or be provided as a hard-coded basic default values in python-api-core library itself.6) Fix the per-call polling config propagation logic (the polling/retry configs supplied to `PollingFuture.result()` used to be ignored for actual call).7) Deprecate the usage of `deadline` terminology in the whole library and backward-compatibly replace it with timeout. This is essential as what has been called "deadline" in this library was actually "timeout" as it is defined in `google.api_core.retry.Retry` class documentation.8) 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.9) 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.10) 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).11) 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).12) Las 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.* fix ci failures (mainly sphinx errors)* remove unused code* fix typo* Pin pytest version to <7.2.0* reformat code* address pr feedback* address PR feedback* address pr feedback* Update google/api_core/future/polling.pyCo-authored-by: Victor Chudnovsky <vchudnov@google.com>* Apply documentation suggestions from code reviewCo-authored-by: Victor Chudnovsky <vchudnov@google.com>* Address PR feedbackCo-authored-by: Victor Chudnovsky <vchudnov@google.com>
parthea added a commit that referenced this pull requestDec 1, 2022
…ch (#474)* fix: Major refactoring of Polling, Retry and Timeout logic (#462)* fix: Major refactoring and fix for Polling, Retry and Timeout logicThis is in response tohttps://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 implementaiton.To properly describe the refactoring this PR does 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 overal confusion among both groups: users of the library and creators of the library. Please check the documentation of the `google.api_core.retry.Retry` class 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 clean as I could make it while still maintaining backward compatibility of the whole library.The quick summary of the changes in this PR:1) 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).2) Properly define RPC Timeout, Retry Timeout and Pollint 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.3) 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.4) 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.5) 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), or be provided as a hard-coded basic default values in python-api-core library itself.6) Fix the per-call polling config propagation logic (the polling/retry configs supplied to `PollingFuture.result()` used to be ignored for actual call).7) Deprecate the usage of `deadline` terminology in the whole library and backward-compatibly replace it with timeout. This is essential as what has been called "deadline" in this library was actually "timeout" as it is defined in `google.api_core.retry.Retry` class documentation.8) 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.9) 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.10) 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).11) 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).12) Las 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.* fix ci failures (mainly sphinx errors)* remove unused code* fix typo* Pin pytest version to <7.2.0* reformat code* address pr feedback* address PR feedback* address pr feedback* Update google/api_core/future/polling.pyCo-authored-by: Victor Chudnovsky <vchudnov@google.com>* Apply documentation suggestions from code reviewCo-authored-by: Victor Chudnovsky <vchudnov@google.com>* Address PR feedbackCo-authored-by: Victor Chudnovsky <vchudnov@google.com>* feat: Allow representing enums with their unqualified symbolic names in headers (#465)* feat: Allow non-fully-qualified enums in routing headers* Rename s/fully_qualified_enums/qualified_enums/g for correctness* chore: minor tweaks* chore: Temporary workaround for pytest in noxfile.* Fix import order* bring coverage to 100%* lint* 🦉 Updates from OwlBot post-processorSeehttps://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md* remove replacement in owlbot.py causing lint failureCo-authored-by: Anthonios Partheniou <partheniou@google.com>Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>* chore(python): update release script dependencies (#472)* chore(python): drop flake8-import-order in samples noxfileSource-Link:googleapis/synthtool@6ed3a83Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:3abfa0f1886adaf0b83f07cb117b24a639ea1cb9cffe56d43280b977033563eb* drop flake8-import-order* lint* use python 3.9 for docs* resolve mypy error* update python version for lint* fix lint* fix lintCo-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>Co-authored-by: Anthonios Partheniou <partheniou@google.com>Co-authored-by: Vadym Matsishevskyi <25311427+vam-google@users.noreply.github.com>Co-authored-by: Victor Chudnovsky <vchudnov@google.com>Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>Co-authored-by: gcf-owl-bot[bot] <78513119+gcf-owl-bot[bot]@users.noreply.github.com>
@release-pleaserelease-pleasebot mentioned this pull requestDec 1, 2022
parthea added a commit to googleapis/gapic-generator-python that referenced this pull requestDec 5, 2022
* fix: Fix timeout default valuesAlso fix LRO for REST transport. This PR makes generated gapics appeciate timeout values from grpc_service_config.json instead of overriding them with None (which means no timeout)It is basically a direct fix for#1477.This PR depends ongoogleapis/python-api-core#462, and expects `setup.py.j2` templates to be updated aftergoogleapis/python-api-core#462 gets pushed and released with new version.* rename uri_prefix to path_prefix to match corresponding python-api-core change* fix unnecessary `gapic_v1.method.DEFAULT` in rest stubs* fix(deps): require google-api-core >=1.34.0* fix(deps): require google-api-core >=2.11.0* revert changes to WORKSPACE* fix typo* fix mypy error* revert local change for debuggingCo-authored-by: Anthonios Partheniou <partheniou@google.com>

try:
kwargs = {} if retry is DEFAULT_RETRY else {"retry": retry}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

FYI: this was a breaking change.#477

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Also for python-aiplatform:googleapis/python-aiplatform#1870

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

that rety logic line never worked (the retry had been i. What broke you is most likely the new default timeout value (instead of None).

copybara-servicebot pushed a commit to kubeflow/pipelines that referenced this pull requestFeb 7, 2023
fix(components): Limit google-api-core version to avoid timeout introduced ingoogleapis/python-api-core#462PiperOrigin-RevId: 507613610
copybara-servicebot pushed a commit to kubeflow/pipelines that referenced this pull requestFeb 9, 2023
fix(components): Limit google-api-core version to avoid timeout introduced ingoogleapis/python-api-core#462PiperOrigin-RevId: 507613610
copybara-servicebot pushed a commit to kubeflow/pipelines that referenced this pull requestFeb 9, 2023
fix(components): Limit google-api-core version to avoid timeout introduced ingoogleapis/python-api-core#462PiperOrigin-RevId: 507613610
copybara-servicebot pushed a commit to kubeflow/pipelines that referenced this pull requestFeb 9, 2023
fix(components): Limit google-api-core version to avoid timeout introduced ingoogleapis/python-api-core#462PiperOrigin-RevId: 507613610
copybara-servicebot pushed a commit to kubeflow/pipelines that referenced this pull requestFeb 9, 2023
fix(components): Limit google-api-core version to avoid timeout introduced ingoogleapis/python-api-core#462PiperOrigin-RevId: 507613610
copybara-servicebot pushed a commit to kubeflow/pipelines that referenced this pull requestFeb 9, 2023
fix(components): Limit google-api-core version to avoid timeout introduced ingoogleapis/python-api-core#462PiperOrigin-RevId: 508463716
@jacobg
Copy link

It seems like this merge broke using freezegun, particularly with google-cloud-datastore in unit testing. Here's the stack trace:

  File "/Users/jacob/.pyenv/versions/gae2/lib/python3.9/site-packages/google/cloud/datastore/transaction.py", line 231, in begin    response_pb = self._client._datastore_api.begin_transaction(  File "/Users/jacob/.pyenv/versions/gae2/lib/python3.9/site-packages/google/cloud/datastore_v1/services/datastore/client.py", line 834, in begin_transaction    response = rpc(  File "/Users/jacob/.pyenv/versions/gae2/lib/python3.9/site-packages/google/api_core/gapic_v1/method.py", line 113, in __call__    return wrapped_func(*args, **kwargs)  File "/Users/jacob/.pyenv/versions/gae2/lib/python3.9/site-packages/google/api_core/timeout.py", line 120, in func_with_timeout    return func(*args, **kwargs)  File "/Users/jacob/.pyenv/versions/gae2/lib/python3.9/site-packages/google/api_core/grpc_helpers.py", line 74, in error_remapped_callable    raise exceptions.from_grpc_error(exc) from excgoogle.api_core.exceptions.DeadlineExceeded: 504 Deadline Exceeded

Would you expect that to happen? Any workarounds?

Mlawrence95 reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tswasttswasttswast left review comments

@vchudnov-gvchudnov-gvchudnov-g left review comments

@atulepatulepatulep approved these changes

@sasha-gitgsasha-gitgsasha-gitg left review comments

@partheapartheaparthea approved these changes

Assignees
No one assigned
Labels
size: xlPull request size is extra large.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

8 participants
@vam-google@atulep@alexander-fenster@jacobg@tswast@parthea@vchudnov-g@sasha-gitg

[8]ページ先頭

©2009-2025 Movatter.jp