fix: check all statuses in check_status to report errors properly - #730
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Connector.check_status() so last_query_ok() can correctly report failures when a multi-command response contains partial errors (previously only the first item was checked, allowing later errors to be missed).
Changes:
- Iterate through all items in list/tuple responses and return the first negative status encountered.
- Preserve prior behavior for all-nonnegative cases by returning the first command’s status.
Comments suppressed due to low confidence (2)
aperturedb/Connector.py:690
check_status’s docstring says it “Returns the status of the first command response”, but this change makes it scan all list items and return the first negative status encountered. Please update the docstring to match the new behavior (and consider fixing the typo “recieved” -> “received” while you’re there) so callers aren’t misled.
for res in json_res:
st = self.check_status(res)
if st < 0:
return st
if len(json_res) > 0:
aperturedb/Connector.py:689
- This change modifies
last_query_ok()behavior for multi-command responses (it can now fail on partial errors). There doesn’t appear to be unit coverage forcheck_status, and most Connector tests rely on an integration DB. Consider adding a small pure unit test that feeds a mixed-status list (e.g., first ok, later negative) intocheck_status/last_query_okto lock in the expected behavior without requiring a live DB.
for res in json_res:
st = self.check_status(res)
if st < 0:
return st
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Addressed the docstring update and added unit tests for |
Fixes #165. When a response with multiple commands (e.g. from querying a large amount of blobs) has partial failures, check_status should return the error status instead of only checking the first command's status.
luisremis
approved these changes
May 23, 2026
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.
Summary
Verification
lenz:55551).Fixes #165.