Skip to content

Retry integration tests on transient service errors#574

Open
ppolewicz wants to merge 5 commits into
masterfrom
ppolewicz/verify-503-integration-retry
Open

Retry integration tests on transient service errors#574
ppolewicz wants to merge 5 commits into
masterfrom
ppolewicz/verify-503-integration-retry

Conversation

@ppolewicz
Copy link
Copy Markdown
Collaborator

This PR verifies integration retry behavior against live B2 CI credentials.\n\nIt adds integration-test retries for transient B2 service errors and an upload test that uses the documented X-Bz-Test-Mode: fail_some_uploads hook.

Copy link
Copy Markdown

@jan-sauer-reef jan-sauer-reef left a comment

Choose a reason for hiding this comment

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

Nice! Unless async integration tests are categorically out of the question, though, I would add a provision in the pytest_pyfunc_call function to await async tests, otherwise they'll pass almost-silently (albeit with a RuntimeWarning).


for attempt in range(INTEGRATION_TEST_RETRY_COUNT + 1):
try:
testfunction(**testargs)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 testfunction isn't async.


from b2sdk._internal.exception import ServiceError, TooManyRequests

RETRYABLE_SERVICE_ERROR_STATUSES = {500, 503}
Copy link
Copy Markdown

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?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants