Fix test retry on throttling#34786
Conversation
| return | ||
|
|
||
| # next_sleep is one of the arguments in the caller | ||
| # sleep_iterator is one of the arguments in the caller |
There was a problem hiding this comment.
I see, the cause of breakage is client change, and we relied on non-public implementation detail on throttlng.
Does the change work with old client library?
There was a problem hiding this comment.
Old version failed with
> self.assertGreater(
container.get_counter(
MetricName('gcsio',
"cumulativeThrottlingSeconds")).get_cumulative(),
1)
E AssertionError: 0 not greater than 1
We'd need to support both, otherwise this means throttling metrics breaks in google-api-core>=2.25.0rc0
There was a problem hiding this comment.
We can inspect compatible google-api-core version in ThrottlingHandler constructor, then at runtime decide which approach to use to extract throttling time.
There was a problem hiding this comment.
Agree！ Let's check the package version before choosing which arg name to access.
I remembered the current approach was the only feasible way to retrieve wait time in an accurate way during my implementation. And I acknowledged it is a bit fragile.
There was a problem hiding this comment.
Added functionality for dynamic selection of the arg name depending on the version of google-api-core
Successful run example with google-api-core >= 2.25.0rc0 - https://github.com/akashorabek/beam/actions/runs/14795779558
Successful run example with google-api-core < 2.25.0rc0 - https://github.com/akashorabek/beam/actions/runs/14796135246
|
Assigning reviewers. If you would like to opt out of this review, comment R: @jrmccluskey for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
3 participants
Fixes: #34736
Successful run example: https://github.com/akashorabek/beam/actions/runs/14753003019
Updated ThrottlingHandler logic based on the new google-api-core release
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.