Add custom request timeout support for VWS and CloudRecoService #2823
Conversation
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
Allow users to configure request timeouts instead of using a hardcoded 30-second default. Both VWS and CloudRecoService now accept an optional request_timeout_seconds parameter during initialization, with proper defaults and documentation. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
Add test_timeout_raises_on_slow_response and test_longer_timeout_succeeds tests to both VWS and CloudRecoService. These tests simulate slow server responses and verify that short timeouts actually raise Timeout exceptions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use keyword arguments for patch.object and pytest.raises to satisfy mypy's type checking. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
adamtheturtle
temporarily deployed
to
development
GitHub Actions
Inactive
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| self._server_access_key = server_access_key | ||
| self._server_secret_key = server_secret_key | ||
| self._base_vws_url = base_vws_url | ||
| self.request_timeout_seconds = request_timeout_seconds |
There was a problem hiding this comment.
Inconsistent attribute naming convention for new property
Low Severity
request_timeout_seconds is stored as a public attribute, but all other constructor parameters (_server_access_key, _server_secret_key, _base_vws_url in VWS; _client_access_key, _client_secret_key, _base_vwq_url in CloudRecoService) are stored as private attributes with _ prefix. Consider using _request_timeout_seconds for consistency, or document why this attribute should be public.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Adds support for configurable request timeouts in both VWS and CloudRecoService APIs. Previously, timeouts were hardcoded to 30 seconds. Now users can specify a custom timeout during client initialization.
Changes
request_timeout_secondsparameter to VWS and CloudRecoService constructors with 30-second defaultTesting
Tests verify that the default timeout is 30 seconds and that custom timeouts can be set and used successfully.
Note
Low Risk
Low risk: keeps the existing 30s default while exposing a new optional timeout knob; behavior changes only for callers that set a custom timeout.
Overview
Adds configurable HTTP request timeouts to both
VWSandCloudRecoServicevia a newrequest_timeout_secondsconstructor parameter (defaulting to the previous hardcoded 30s).Plumbs the timeout through to the underlying
requests.requestcalls (including_target_api_request) and adds tests covering default/custom values plus simulated slow-response timeout/success behavior.Written by Cursor Bugbot for commit a0b073d. This will update automatically on new commits. Configure here.