-
Notifications
You must be signed in to change notification settings - Fork 65
Retry integration tests on transient service errors #574
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
base: master
Are you sure you want to change the base?
Changes from all commits
250f86b
9db5f07
84bcfe0
2ec8bc2
8a1001a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Retry integration tests automatically when they fail because of a transient HTTP 503 from B2. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,9 +7,44 @@ | |
| # License https://www.backblaze.com/using_b2_code.html | ||
| # | ||
| ###################################################################### | ||
| from __future__ import annotations | ||
|
|
||
| import pytest | ||
|
|
||
| from b2sdk._internal.exception import ServiceError, TooManyRequests | ||
|
|
||
| RETRYABLE_SERVICE_ERROR_STATUSES = {500, 503} | ||
| INTEGRATION_TEST_RETRY_COUNT = 4 | ||
|
|
||
|
|
||
| @pytest.fixture(scope='session', autouse=True) | ||
| def auto_change_account_info_dir(change_account_info_dir): | ||
| pass | ||
|
|
||
|
|
||
| @pytest.hookimpl(tryfirst=True) | ||
| def pytest_pyfunc_call(pyfuncitem): | ||
| testfunction = pyfuncitem.obj | ||
| funcargs = pyfuncitem.funcargs | ||
| testargs = {arg: funcargs[arg] for arg in pyfuncitem._fixtureinfo.argnames} | ||
|
|
||
| for attempt in range(INTEGRATION_TEST_RETRY_COUNT + 1): | ||
| try: | ||
| testfunction(**testargs) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are no async tests but if there ever were in the future, they would never be awaited and unless the test runner sees the runtime warning, they'd never know -> they'd appear to always pass. It might be worth checking that
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is not a shred of |
||
| return True | ||
| except ServiceError as exc: | ||
| if exc._status not in RETRYABLE_SERVICE_ERROR_STATUSES: | ||
| raise | ||
| if attempt >= INTEGRATION_TEST_RETRY_COUNT: | ||
| raise | ||
| print( | ||
| f'Retrying {pyfuncitem.nodeid} after transient service error {exc._status}:' | ||
| f' attempt {attempt + 1} of {INTEGRATION_TEST_RETRY_COUNT}' | ||
| ) | ||
| except TooManyRequests: | ||
| if attempt >= INTEGRATION_TEST_RETRY_COUNT: | ||
| raise | ||
| print( | ||
| f'Retrying {pyfuncitem.nodeid} after transient too many requests:' | ||
| f' attempt {attempt + 1} of {INTEGRATION_TEST_RETRY_COUNT}' | ||
| ) | ||
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.
Tests are retried on 500 errors but the bucket operations in
_retry_bucket_test_operation()only on 503.a) Is nested retry logic necessary for a test?
b) If so, shouldn't the BucketManager also retry on 500 errors then?
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.
you are right, it should always retry on 5xx