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

Deprecation warning breaks CGI scripts #998

Closed
bkline opened this issue Jan 1, 2022 · 6 comments · Fixed by #1002
Closed

Deprecation warning breaks CGI scripts #998

bkline opened this issue Jan 1, 2022 · 6 comments · Fixed by #1002

Comments

@bkline
Copy link
Contributor

bkline commented Jan 1, 2022

Environment

  • Python: 3.10.1
  • pyodbc: 4.0.32
  • OS: Any
  • DB: Any
  • driver: Any

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 added a commit to bkline/pyodbc that referenced this issue Jan 2, 2022
…er#998)

Current versions of Python write a deprecation warning message to
stderr, which breaks CGI scripts running under web servers which
fold stderr into stdout. Likely breaks other software. This change
replaces the deprecated calls with PyUnicode_New(size, 0x10ffff).
The second argument represents the highest code point for Unicode
characters, which is used because the calls being replaced have
no string value from which we can determine the smallest number
of bytes needed to represent any value which can be stored in the
newly-created object.
bkline added a commit to bkline/pyodbc that referenced this issue Jan 2, 2022
…er#998)

Current versions of Python write a deprecation warning message to
stderr, which breaks CGI scripts running under web servers which
fold stderr into stdout. Likely breaks other software. This change
replaces the deprecated calls with PyUnicode_New(size, 0x10ffff).
The second argument represents the highest code point for Unicode
characters, which is used because the calls being replaced have
no string value from which we can determine the smallest number
of bytes needed to represent any value which can be stored in the
newly-created object.
bkline added a commit to bkline/pyodbc that referenced this issue Jan 2, 2022
…er#998)

Current versions of Python write a deprecation warning message to
stderr, which breaks CGI scripts running under web servers which
fold stderr into stdout. Likely breaks other software. This change
replaces the deprecated calls with PyUnicode_New(size, 0x10ffff).
The second argument represents the highest code point for Unicode
characters, which is used because the calls being replaced have
no string value from which we can determine the smallest number
of bytes needed to represent any value which can be stored in the
newly-created object.
@bkline
Copy link
Contributor Author

bkline commented Jan 2, 2022

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
Copy link
Collaborator

Related issue created by @keitherskine - #917

bkline added a commit to bkline/pyodbc that referenced this issue Jan 2, 2022
…er#998)

Current versions of Python write a deprecation warning message to
stderr, which breaks CGI scripts running under web servers which
fold stderr into stdout. Likely breaks other software. This change
replaces the deprecated calls with PyUnicode_New(size, maxchar).
The accompanying code to populate the new objects has also been
rewritten to use the new PyUnicode APIs.
@bkline
Copy link
Contributor Author

bkline commented Jan 2, 2022

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
Copy link

vfantin commented Jan 4, 2022

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
Copy link
Contributor Author

bkline commented Jan 4, 2022

@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
Copy link
Collaborator

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.

mkleehammer pushed a commit that referenced this issue Jul 14, 2022
Current versions of Python write a deprecation warning message to
stderr, which breaks CGI scripts running under web servers which
fold stderr into stdout. Likely breaks other software. This change
replaces the deprecated calls with PyUnicode_New(size, maxchar).
The accompanying code to populate the new objects has also been
rewritten to use the new PyUnicode APIs.
v-makouz pushed a commit to v-makouz/pyodbc that referenced this issue Sep 9, 2022
* Add support for Python 3.10, drop EOL 3.5 (mkleehammer#952)

* Remove duplicate entry in pyi stub (mkleehammer#979)

* Replace deprecated SafeConfigParser with ConfigParser (mkleehammer#953)

* Designate connection string as optional (mkleehammer#987)

* Fix spelling typos (mkleehammer#985)

Co-authored-by: Gord Thompson <[email protected]>

* Fix for DSN Names with non-ASCII chars (mkleehammer#951)

* Fix for DSN Names with non-ASCII chars

Fixes: mkleehammer#948

Co-authored-by: bamboo <[email protected]>
Co-authored-by: Gord Thompson <[email protected]>

* Added InterfaceError to pyodbc.pyi. (mkleehammer#1013)

Co-authored-by: Benjamin Holder <[email protected]>

* Upgrade deprecated unicode encoding calls (mkleehammer#792)

* Do not include .pyc artifacts in source tarball mkleehammer#742

* Build wheels with cibuildwheels on GitHub Actions

Fixes mkleehammer#175
Ref mkleehammer#688
Closes mkleehammer#668
Closes mkleehammer#685
Fixes mkleehammer#441 and pretty much most issues that mention
` sql.h: No such file or directory`

This also need to setup some PyPI keys for automated uploads.

* Install unixodbc-dev for Linux wheels

* Enable GitHub Actions for pull requests

* Use Debian based `manylinux_2_24` image

* `apt-get` update before installing in wheel build

* Use PEP 440 version name required for wheels

* Skip building 32-bit wheels

* 4.0.dev0 for default version, because test_version() wants 3 parts here

Checked this won't shadow released minor version (credit goes to @hugovk)

    >>> from packaging.version import Version
    >>> Version("4.0.dev0") > Version("4.0.24")
    False

* Had to use Debian image for PyPy too

* Disable PyPy wheels

https://cibuildwheel.readthedocs.io/en/stable/options/#build-selection

PyPy is missing some C functions that `pyodbc` needs.

* Update README.md

* Avoid error when testing with DSN= connection

Fixes: mkleehammer#1000

* Disable setencoding/setdecoding in tests3/pgtests.py

Fixes: mkleehammer#1004

* Adjust test_columns() in tests3/pgtests.py for newer driver versions

Fixes: mkleehammer#1003

* Move driver version check out of function

* Add comment to _get_column_size()

* Fix memory leak with decimal parameters

Fixes: mkleehammer#1026

* Create codeql-analysis.yml

* Bugfix/sql param data memory leak (mkleehammer#703)

* Updated .gitignore

* * Created a test file for the specific scenario

* * Updated doc of test file for the specific SQLParamData scenario

* * Fixed the test file for the specific SQLParamData scenario by Py_XDECREF the PyObject with 1 reference.

* * Improved the test to close the cursor and set it to None, then forcing the gc

* * Changed the fix of the memory leak and updated the test.

* * Removed redundant empty line

* * Converted tabs to spaces

* * Moved variable out of conn's scope

* Update gitignore, remove duplicated

* Replace deprecated PyUnicode_FromUnicode(NULL, size) calls (mkleehammer#998)

Current versions of Python write a deprecation warning message to
stderr, which breaks CGI scripts running under web servers which
fold stderr into stdout. Likely breaks other software. This change
replaces the deprecated calls with PyUnicode_New(size, maxchar).
The accompanying code to populate the new objects has also been
rewritten to use the new PyUnicode APIs.

* Making pyodbc compatible with PostgreSQL infinity dates, returning MINYEAR and MAXYEAR to python, instead of values out of python's limits

* Removing autoformat from code

* Removing autoformat from code

* Add odbc_config support on mac and m1 homebrew dir

* Note EOL of 2.7 support in README (mkleehammer#945)

* Fix version of CI generated wheels

The CI system is checking out exact tags like "git checkout 4.0.33", which results in a
detached HEAD.  The version calculation was adding the commit hash.

* Fix for mkleehammer#1082 libraries in Linux wheels (mkleehammer#1084)

* use argparse instead of optparse (mkleehammer#1089)

Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Alex Nelson <[email protected]>
Co-authored-by: Kian Meng, Ang <[email protected]>
Co-authored-by: Gord Thompson <[email protected]>
Co-authored-by: bamboo <[email protected]>
Co-authored-by: Gord Thompson <[email protected]>
Co-authored-by: bdholder <[email protected]>
Co-authored-by: Benjamin Holder <[email protected]>
Co-authored-by: Inada Naoki <[email protected]>
Co-authored-by: Michael Fladischer <[email protected]>
Co-authored-by: Anatoli Babenia <[email protected]>
Co-authored-by: Francisco Morales <[email protected]>
Co-authored-by: Gord Thompson <[email protected]>
Co-authored-by: Michael Kleehammer <[email protected]>
Co-authored-by: Gilad Leifman <[email protected]>
Co-authored-by: Bob Kline <[email protected]>
Co-authored-by: Leandro Scott <[email protected]>
Co-authored-by: Jordan Mendelson <[email protected]>
Co-authored-by: Keith Erskine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants