mkleehammer / pyodbc

Python ODBC bridge
https://github.com/mkleehammer/pyodbc/wiki
MIT No Attribution
2.92k stars 563 forks source link

Deprecation warning breaks CGI scripts #998

Closed bkline closed 2 years ago

bkline commented 2 years ago

Environment

Issue

Under Python 3.10 PyUnicode_FromUnicode(NULL, size) triggers a DeprecationWarning message to stderr, breaking CGI scripts. To reproduce:

#!/usr/bin/env python3

"""Repro case to demonstrate deprecation warning.

Breaks CGI scripts.
"""

from argparse import ArgumentParser
from pyodbc import connect  # pylint: disable=no-name-in-module

parser = ArgumentParser()
parser.add_argument("--connection-string", "-c", required=True)
opts = parser.parse_args()
conn = connect(opts.connection_string)
cursor = conn.cursor()
cursor.execute("SELECT 1 AS a, 2 AS b")
rows = cursor.fetchall()
print(f"""\
Content-type: text/plain

HTTP header(s) should be first in the output, but they're not. :(
{rows!r}""")

Expected Output

Content-type: text/plain

HTTP header(s) should be first in the output, but they're not. :(
[(1, 2)]

Actual Output

/Users/bkline/gdrive/Notes/Python/./pyodbcrepro.py:18: DeprecationWarning: PyUnicode_FromUnicode(NULL, size) is deprecated; use PyUnicode_New() instead
  print(f"""\
Content-type: text/plain

HTTP header(s) should be first in the output, but they're not. :(
[(1, 2)]
bkline commented 2 years ago

OK, I'm going to need help from someone more familiar with the bowels of the Python internals than I. Unless I just need new glasses, this message describing the failure test_row_repr() in tests3/sqlservertests.py running against the PR's code just seems wrong.

======================================================================
FAIL: test_row_repr (__main__.SqlServerTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/bkline/repos/pyodbc/tests3/sqlservertests.py", line 1203, in test_row_repr
    self.assertEqual(result, "(1, 2, 3, 4)")
AssertionError: '(1, 2, 3, 4)' != '(1, 2, 3, 4)'
- (1, 2, 3, 4)
+ (1, 2, 3, 4)

Clearly, there's more to the story than is conveyed by the deprecation message ...

DeprecationWarning: PyUnicode_FromUnicode(NULL, size) is deprecated; use PyUnicode_New() instead

... and the documentation for PyUnicode_New().

gordthompson commented 2 years ago

Related issue created by @keitherskine - #917

bkline commented 2 years ago

First attempt at a PR didn't pass the pipeline tests, so I re-rolled a fresh PR and this one passes all the tests. This version, instead of just doing what the deprecation warning instructed, was a complete rewrite of the two functions which called PyUnicode_FromUnicode(NULL, size). It would be good to get some eyes on this much more experienced than mine with the Python C API.

vfantin commented 2 years ago

Hi everyone,

I had warning messages when I print(rows) in my python code, so I checked the Row_repr code in the last few days. I was arrived at the same conclusion and rewrote the function in the same way you did … but after that I asked myself if it was really necessary. At the end we just want the Row_repr function print a tuple .. so I’m not sure if we really have to manage the unicode in the Row_repr function. I rewrote the function like this :

static PyObject* Row_repr(PyObject* o)
{
    Row* self = (Row*)o;

    if (self->cValues == 0)
        return PyString_FromString("()");

    Tuple self_tuple(PyTuple_New(self->cValues));
    if (!self_tuple.IsValid())
        return 0;

    for (Py_ssize_t i = 0; i < self->cValues; i++)
        PyTuple_SET_ITEM(self_tuple.Get(),i,self->apValues[i]);

    return PyObject_Repr(self_tuple);
}

This function passes all sqlserver tests on my Windows with Python 3.10.

It's probably too simple ... but I'm curious whether should we really handle the unicode characters in the Row_repr function?

bkline commented 2 years ago

@vfantin I don't see why your approach wouldn't work for Row_repr(). I'm a little embarrassed that I didn't think of it myself. That technique can't be used for the other call to PyUnicode_FromUnicode(NULL, size) (in MakeConnectionString()), though.

gordthompson commented 2 years ago

As a workaround until #1002 is accepted, we can run our python script using

python -W ignore::DeprecationWarning my_script.py

or set an environment variable named PYTHONWARNINGS to the value ignore::DeprecationWarning and then just run the script normally.