Skip to content
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

Connection.close() doesn't close cursors #461

Closed
IlyaFaer opened this issue Aug 20, 2020 · 0 comments · Fixed by #463
Closed

Connection.close() doesn't close cursors #461

IlyaFaer opened this issue Aug 20, 2020 · 0 comments · Fixed by #463
Assignees
Labels
api: spanner Issues related to the googleapis/python-spanner-django API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@IlyaFaer
Copy link
Contributor

According to PEP-249, on connection closing all the related cursors must become no-op as well. The current implementation of closes and checks has an issue at this point.

Connection.close() only sets the connection attribute to True:

def close(self):
self.rollback()
self.__dbhandle = None
self._closed = True

While the Cursor checking method is only looking for the Cursor._closed attribute:

def _raise_if_already_closed(self):
"""
Raise an exception if attempting to use an already closed connection.
"""
if self._closed:
raise InterfaceError('cursor already closed')

That means cursor don't know if the related connection is already closed. Technically, it doesn't look like a big problem, as Cursor.execute() delegates to Connection, and the connection will raise an exception already closed. But there are several commands, which will be runned on a closed Cursor before the exception raised, for example, classify_stmt(sql).

To avoid that it'll be good to add into Cursor._raise_if_already_closed() a line like:

if self._connection.is_closed:
    raise ...

Checking a bool is much more faster than regexping SQL code and passing it into a connection method.

Plus to this, I'd make is_closed attribute public, so that users could check if a cursor/connection is closed.

@IlyaFaer IlyaFaer added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: spanner Issues related to the googleapis/python-spanner-django API. labels Aug 20, 2020
@IlyaFaer IlyaFaer self-assigned this Aug 20, 2020
@c24t c24t closed this as completed in #463 Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/python-spanner-django API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant