-
Notifications
You must be signed in to change notification settings - Fork 48
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: allow RetryStrategy to be configured with a custom initial wait and multiplier #216
Conversation
213500e
to
a18c79d
Compare
@tseaver thanks, PTAL |
a18c79d
to
f2f01f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks! @frankyn FYI as well.
@@ -195,6 +203,41 @@ def test_success_with_retry(self, randint_mock, sleep_mock): | |||
sleep_mock.assert_any_call(2.625) | |||
sleep_mock.assert_any_call(4.375) | |||
|
|||
@mock.patch(u"time.sleep") | |||
@mock.patch(u"random.randint") | |||
def test_success_with_retry_custom_delay(self, randint_mock, sleep_mock): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice test!
base_wait, wait_time = calculate_retry_wait(base_wait, retry_strategy.max_sleep) | ||
base_wait, wait_time = calculate_retry_wait( | ||
base_wait, retry_strategy.max_sleep, retry_strategy.multiplier | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of surprising that this is all calculated in the library rather than using a helper (in Go we use gax) but obviously nothing to address here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strong agree. We have such a helper for python-storage but because the transport layer is different I couldn't simply replace the implementation here with that helper. In the future I'd like to do a bit more work to bring resumable media into the fold in terms of unifying its behavior with python-storage and BQ.
This is a backwards-compatible change to allow the python-storage client to configure retries in media calls similarly to how it configures retries in API calls.