Bug Retrying Writes#40672
Merged
FabianMeiswinkel merged 5 commits intoAzure:mainfrom Apr 24, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a bug in the Cosmos DB retry policy where patch and replace operations were being retried unexpectedly and ensures that CosmosHttpResponseError is raised immediately. It also adds tests for both asynchronous and synchronous behaviors, updates fault injection transport to use header constants, and updates the changelog accordingly.
- Added async and sync tests (test_patch_replace_no_retry) to verify that patch/replace operations do not retry.
- Modified the retry policies in both async and sync modules to immediately raise CosmosHttpResponseError.
- Updated fault injection transports to use constant header values and include a new predicate for operation type.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/cosmos/azure-cosmos/tests/test_retry_policy_async.py | Added async test for patch/replace no retry behavior. |
| sdk/cosmos/azure-cosmos/tests/test_retry_policy.py | Added sync test for patch/replace no retry behavior. |
| sdk/cosmos/azure-cosmos/tests/test_config.py | Updated MockConnectionRetryPolicy signature and error handling logic. |
| sdk/cosmos/azure-cosmos/tests/_fault_injection_transport_async.py | Updated header usage to constants and added fault injection predicate for operation type. |
| sdk/cosmos/azure-cosmos/tests/_fault_injection_transport.py | Updated header usage to constants and added fault injection predicate for operation type. |
| sdk/cosmos/azure-cosmos/azure/cosmos/aio/_retry_utility_async.py | Modified retry logic to immediately raise CosmosHttpResponseError. |
| sdk/cosmos/azure-cosmos/azure/cosmos/_retry_utility.py | Modified retry logic to immediately raise CosmosHttpResponseError. |
| sdk/cosmos/azure-cosmos/CHANGELOG.md | Documented the bug fix for not retrying patch/replace operations. |
Collaborator
|
API change check API changes are not detected in this pull request. |
Member
Author
|
/azp run python - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
cRui861
pushed a commit
that referenced
this pull request
May 14, 2025
* No retries for writes * update changelog * fix pylint * fix test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Bug
We were still relying on azure core's
is_method_retryableto retryAzureError. There are two issues with using this method. This method will behave differently based on customers passed in options. A customer could change it so that creates are retried. Secondly by default, this method retries patch and replace operations. All these retries are after the request has been sent and certain 5xx status codes are returned.Solution
Instead of handling
CosmosHttpResponseErrorsin the connection retry policy, they will immediately be raised. I kept the AzureError code, but unsure when this code path will be executed. If there is some unexpected AzureError, we would retry but only if it is a read request.Other Changes