aws / amazon-redshift-python-driver

Redshift Python Connector. It supports Python Database API Specification v2.0.
Apache License 2.0
202 stars 72 forks source link

SQL errors cause UserWarning: DB-API extension cursor.connection used self._run_cursor.execute(sql, params, stream=stream) to be displayed #204

Closed patatwork365 closed 4 months ago

patatwork365 commented 5 months ago

Driver version

redshift-connector 2.0.917

Redshift version

PostgreSQL 8.0.2 on i686-pc-linux-gnu, compiled by GCC gcc (GCC) 3.4.2 20041017 (Red Hat 3.4.2-6.fc3), Redshift 1.0.61626�

Client Operating System

Linux cc7c404a8a4d 6.5.11-linuxkit #1 SMP PREEMPT Wed Dec 6 17:08:31 UTC 2023 x86_64 GNU/Linux

Python version

3.11.7

Table schema

Problem description

When a SQL error occurs the warning 'UserWarning: DB-API extension cursor.connection used self._run_cursor.execute(sql, params, stream=stream)' is logged.

Catching the raised exception does not suppress the warning

Python Driver trace logs

Reproduction code

import redshift_connector
from getpass import getpass
from pprint import pprint as pp

host = 'the-host'
database = 'the-db'
user = 'the-user'
password = getpass('''What's the magic word! : ''')

def run_sql(*, conn, sql):
    pp(sql)
    rtn = None
    try:
        rtn = conn.run(sql)
    except Exception as e:
        pp(str(e))
        conn.rollback()

    return rtn

conn = redshift_connector.connect(host=host, database=database, user=user, password=password)

sql_list = [
    '''select 'I`m a teapot' ''', 
    '''respond 'You`re a teapot' ''',  # invalid sql of your choice
    '''select 'Who`s a teapot' ''', 
    '''respond 'You`re a teapot' '''
]

res = [run_sql(conn=conn, sql=sql) for sql in sql_list]
pp(res)

Output

What's the magic word! : "select 'Im a teapot' " "respond 'Youre a teapot' " /home/vscode/.local/lib/python3.11/site-packages/redshift_connector/core.py:1321: UserWarning: DB-API extension cursor.connection used self._run_cursor.execute(sql, params, stream=stream) ("{'S': 'ERROR', 'C': '42601', 'M': 'syntax error at or near " '"respond"\', \'P\': \'1\', \'F\': ' "'/home/ec2-user/padb/src/pg/src/backend/parser/parser_scan.l', 'L': '840', " "'R': 'yyerror'}") "select 'Whos a teapot' " "respond'Youre a teapot' " ("{'S': 'ERROR', 'C': '42601', 'M': 'syntax error at or near " '"respond"\', \'P\': \'1\', \'F\': ' "'/home/ec2-user/padb/src/pg/src/backend/parser/parser_scan.l', 'L': '840', " "'R': 'yyerror'}") [(['Im a teapot'],), None, (['Whos a teapot'],), None]

Brooke-white commented 5 months ago

Hi @patatwork365 , thank you for reporting this issue. This warning is due to the error handling logic within cursor.execute() method accessing the read-only connection attribute on the cursor. Within this error handling logic we perform some logging of the connection's socket state in order to help us determine if the execute methods exception was due to a socket issue such as network timeout. This is helpful for cases where a user is hitting a timeout/failure for unknown reason and it's suspected that there's an issue with redshift-connector.

A fix is needed to address this behavior where we retain the current logging capabilities but no longer raise this warning. From a quick look, this fix is likely as simple from changing the logging calls from:

_logger.debug("Cursor's connection._usock state: %s", self.connection._usock.__dict__)  # type: ignore
_logger.debug("Cursor's connection._sock is closed: %s", self.connection._sock.closed)  # type: ignore

to

_logger.debug("Cursor's connection._usock state: %s", self._c._usock.__dict__)  # type: ignore
_logger.debug("Cursor's connection._sock is closed: %s", self._c._sock.closed)  # type: ignore

as Cursor.connection raises a warning while Cursor._c does not as it's indicated for use internal to the Cursor class. Please let me know if you (or someone else in the community) would like to raise a PR with this fix, otherwise I will fix and anticipate it will be included in our next release.

patatwork365 commented 5 months ago

Thanks @Brooke-white for the analysis. I'm not currently able to contribute as only have access from a work laptop ATM and they have the secret police following us ...

Brooke-white commented 5 months ago

Thanks, @patatwork365 . I'll give it a few days to see if anyone else would like to contribute and will raise a PR myself otherwise.

Brooke-white commented 5 months ago

Hi folks, this fix will be in our February release

Brooke-white commented 4 months ago

Hi @patatwork365 , we've fixed this in 2.1.0 thanks for your patience :)