fix: honoring read_timeout at the cosmos client level#44472
Conversation
|
/azp run python - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR implements client-level read timeout configuration for Azure Cosmos DB operations. The fix ensures that when a read timeout is specified at the client level, it automatically applies to all queries and operations unless explicitly overridden at the request level.
Key Changes
- Added read timeout handling in CosmosClient initialization to propagate client-level timeouts to the connection policy
- Extended container property getters to pass through read_timeout options from request level
- Added comprehensive test coverage for client-level, request-level, and policy-level timeout behaviors
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/cosmos/azure-cosmos/azure/cosmos/cosmos_client.py | Added logic to extract and apply client-level read_timeout to ConnectionPolicy |
| sdk/cosmos/azure-cosmos/azure/cosmos/aio/_cosmos_client.py | Added async version of client-level read_timeout handling |
| sdk/cosmos/azure-cosmos/azure/cosmos/container.py | Extended _get_properties_with_options to propagate read_timeout from options |
| sdk/cosmos/azure-cosmos/azure/cosmos/aio/_container.py | Extended async _get_properties_with_options to propagate read_timeout |
| sdk/cosmos/azure-cosmos/tests/test_crud.py | Added three test methods covering request-level override, client-level timeout, and policy-level timeout scenarios |
| sdk/cosmos/azure-cosmos/tests/test_crud_async.py | Added async versions of the three timeout test methods |
|
@dibahlfi Another comment not directly related to this PR. Shouldn't Also, please make both |
|
Hi @dibahlfi. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
github-actions
bot
added
the
no-recent-activity
|
Hi @dibahlfi. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing |
|
/reopen |
github-actions
bot
removed
the
no-recent-activity
|
/azp run python - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
allenkim0129
left a comment
allenkim0129
left a comment
There was a problem hiding this comment.
Thank you for addressing comments, and the updated the codes looks good, but I just have some comments on testing.
tvaron3
left a comment
tvaron3
left a comment
There was a problem hiding this comment.
PR Deep Review — Single Finding
Posting one observation on the docstring type.
|
/azp run python - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - cosmos - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The fix enables client-level read timeout configuration to automatically applies to all queries unless explicitly specified at the request level.