| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Signed-off-by: Nanook <nanookclaw@users.noreply.github.com>
There was a problem hiding this comment.
Thanks for the contribution, would be great if we can address the comments and get this in
Sorry, something went wrong.
| elif self.active_command_id is not None: | ||
| self.backend.close_command(self.active_command_id) |
There was a problem hiding this comment.
Sorry, something went wrong.
There was a problem hiding this comment.
ResultSet.close() gates the RPC on status != CLOSED and not has_been_closed_server_side and connection.open (result_set.py:180-182). The new branch fires unconditionally, sending a pointless (and, per F1, unguarded) close_command RPC even when the session is already closed server-side. Gating on self.connection.open would avoid the round-trip and much of F1's trigger surface.
Sorry, something went wrong.
There was a problem hiding this comment.
Both added tests use bare Mock() backends whose close_command never raises, so they only prove the happy case. The one genuinely new invariant — active_command_id still cleared (via finally) and close() staying best-effort when close_command raises — is entirely unverified. Fix: add a test with mock_backend.close_command.side_effect = RequestError(...), asserting close() does not raise and active_command_id is None.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
Cursor.close() now frees an active async command id when no result set has been fetched yet. This covers the execute_async() followed by close() path from #791, where active_command_id is set but active_result_set is still None.
When a result set exists, close still delegates to active_result_set.close() so that path can perform its existing backend cleanup without a duplicate close_command call. The cursor clears active_command_id in a finally block either way.
Tests
Closes #791