Closing a connection tries to close already closed cursors

[betodealmeida]

When closing a connection the library will call .close() on every cursor created for that connection, including closed ones. While the code works, it produces a lot of logs of error level, polluting the logs.

Environment details

• OS type and version: Mac OS 10.15.7
• Python version: Python 3.8.2
• pip version: pip 20.3.3
• google-cloud-bigquery version: 2.7.0

Steps to reproduce

1. Close a cursor
2. Close the connection
3. Error log shows Exception closing connection <google.cloud.bigquery.dbapi.connection.Connection object at 0x...>

Code example

from contextlib import closing

# using pybigquery
with closing(engine.raw_connection()) as conn:
    with closing(conn.cursor()) as cursor:
        cursor.execute(sql)

Stack trace

Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/pool/base.py", line 270, in _close_connection
    self._dialect.do_close(connection)
  File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 549, in do_close
    dbapi_connection.close()
  File "/usr/local/lib/python3.7/site-packages/google/cloud/bigquery/dbapi/_helpers.py", line 258, in with_closed_check
    return method(self, *args, **kwargs)
  File "/usr/local/lib/python3.7/site-packages/google/cloud/bigquery/dbapi/connection.py", line 79, in close
    cursor_.close()
  File "/usr/local/lib/python3.7/site-packages/google/cloud/bigquery/dbapi/_helpers.py", line 257, in with_closed_check
    raise exc_class(exc_msg)
google.cloud.bigquery.dbapi.exceptions.ProgrammingError: Operating on a closed cursor.

Suggested fix

# google/cloud/bigquery/dbapi/connection.py
class Connection(object):
    ...
    def close(self):
        ...
        for cursor_ in self._cursors_created:
            if not cursor_._closed:
                cursor_.close()

[plamut]

@betodealmeida Thanks for the report.

I can see where the issue is - the raise_on_closed() decorator makes all public methods throw if an instance is "closed", which includes the Cursor.close() method. Additionally, when closing a connection, the latter also attempts to automatically close all cursors that it created, thus it's not necessary to nest the closing() context managers:

with closing(engine.raw_connection()) as conn:
    conn.cursor().execute(sql)

However, there might be valid reasons to close a cursor early, and I agree that closing the connection at some later point should behave more gracefully, I'll fix that.

Edit: Ah, you already submitted a PR, even better, thanks!