-
Notifications
You must be signed in to change notification settings - Fork 112
Asyncio vector deletion error #577
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: main
Are you sure you want to change the base?
Conversation
…se types Co-authored-by: jhamon <[email protected]>
|
Cursor Agent can help with this pull request. Just |
|
@cursoragent seems like some lint errors |
|
Perfect! I've fixed all the lint errors. The issues were in the test file: Fixed:
Changes committed and pushed:
The linter should now pass on CI. All tests still pass with these changes. Learn more about Cursor Agents |
- Remove unused MagicMock import - Remove unused result variable assignments in string response tests
Wrap test logic in try-finally block to ensure AsyncioApiClient is properly closed even if assertions fail. This makes it consistent with the other two async tests in the same file.
|
@cursoragent can we add an integration test that verifies async delete vector is working |
|
Excellent! I've added comprehensive integration tests for async vector deletion. Here's what I created: New Integration Test File
Test Coverage
Key Features✅ Tests the exact scenario from GitHub issue #564 Committed and pushed: These integration tests will run in CI and verify that the async delete operations work correctly end-to-end, ensuring the bug fix is effective in real-world scenarios! |
Add comprehensive integration tests to verify async delete operations work correctly: - test_delete_by_ids: Delete specific vectors by ID - test_delete_all_in_namespace: Delete all vectors (the original bug scenario) - test_delete_by_filter: Delete vectors matching a filter - test_delete_response_has_response_info: Verify _response_info is present These tests ensure the AttributeError fix works correctly in real scenarios. Related to #564
Fixes
AttributeErrorwhen assigning_response_infoto primitive types in API responses, particularly for asyncio delete operations.GitHub Issue
Closes #564
Problem Description
When using the asyncio SDK to delete vectors, the following error occurs:
The error happens at:
pinecone/openapi_support/asyncio_api_client.py, line 182pinecone/openapi_support/api_client.py, line 217 (sync version has the same bug)Root Cause
The issue occurs in the code that attaches response metadata (
_response_info) to API responses. The code attempts to set_response_infoon the return data using one of two approaches:_response_infoas a dictionary key_response_infoas an attribute usingsetattr()However, the code doesn't handle primitive types (str, int, float, bool, bytes, None) which don't support attribute assignment. If the API returns or the deserializer produces a primitive type, the
setattr()call fails with anAttributeError.The
delete()operation uses_check_type=Falseby default, which may allow the deserializer to return unexpected types in certain edge cases or API response scenarios.Solution
Modified both
asyncio_api_client.pyandapi_client.pyto handle primitive types gracefully:Before:
After:
The fix:
setattr()_response_infoon primitive types (they can't have it anyway)Testing
New Tests Added
Created comprehensive unit tests in
tests/unit/test_response_info_assignment.py:_response_infoas a dictionary key_response_infoas an attributeExisting Tests
All 367 existing unit tests pass with the fix applied.
Files Changed
Impact
Linear Issue: PIN-12
Note
Ensures response metadata attachment doesn’t crash on primitive responses and validates delete flows in asyncio.
pinecone/openapi_support/api_client.pyandpinecone/openapi_support/asyncio_api_client.pyto only set_response_infoondictor model instances; add primitive-type check andsetattrtry/except guardtests/integration/rest_asyncio/db/data/test_delete.pycovering delete by IDs, delete-all, and delete-by-filter in asyncio, asserting dict response and_response_infopresencetests/unit/test_response_info_assignment.pyverifying behavior for dict, string, None, and model responses (sync and asyncio variants)Written by Cursor Bugbot for commit 817beb1. This will update automatically on new commits. Configure here.