PYTHON-2001 Fix warnings emitted by Python 3.8 #428

ShaneHarvey · 2019-11-01T21:34:48Z

Fix DeprecationWarning: PY_SSIZE_T_CLEAN will be required for '#' formats
Fix DeprecationWarning: isAlive() is deprecated, use is_alive() instead
Fix SyntaxWarning: invalid escape sequence
Test Python 3.8 on Travis

https://jira.mongodb.org/browse/PYTHON-2001

Fix DeprecationWarning: PY_SSIZE_T_CLEAN will be required for '#' formats Fix DeprecationWarning: isAlive() is deprecated, use is_alive() instead Fix SyntaxWarning: invalid escape sequence Test Python 3.8 on Travis

ShaneHarvey · 2019-11-01T21:37:05Z

bson/_cbsonmodule.c

@@ -20,6 +20,7 @@
 * should be used to speed up BSON encoding and decoding.
 */

+#define PY_SSIZE_T_CLEAN


See python/cpython@0895793 and https://docs.python.org/3.8/c-api/arg.html#strings-and-buffers for an explanation of the PY_SSIZE_T_CLEAN changes.

PEP 353 -- Using ssize_t as the index type is also worth a read. The "Conversion guidelines" section in particular. I've gone ahead and fixed the conversion warnings by using the 3 option: "otherwise, check whether the value fits an int, and raise a ValueError if it doesn't."

ShaneHarvey · 2019-11-01T23:08:29Z

I still need to fix these test failures on Windows:

 [2019/11/01 15:45:11.110] ERROR [0.000s]: test_encode_decode_roundtrip (test_custom_types.TestCustomPythonBSONTypeToBSONMonolithicCodec)
 [2019/11/01 15:45:11.110] ----------------------------------------------------------------------
 [2019/11/01 15:45:11.110] Traceback (most recent call last):
 [2019/11/01 15:45:11.110]   File "C:\data\mci\da86b3c3334c0582b80d293273f2d335\src\test\test_custom_types.py", line 140, in test_encode_decode_roundtrip
 [2019/11/01 15:45:11.110]     self.roundtrip({'average': [[Decimal('56.47')]]})
 [2019/11/01 15:45:11.167]   File "C:\data\mci\da86b3c3334c0582b80d293273f2d335\src\test\test_custom_types.py", line 133, in roundtrip
 [2019/11/01 15:45:11.167]     rt_document = decode(bsonbytes, codec_options=self.codecopts)
 [2019/11/01 15:45:11.167]   File "C:\data\mci\da86b3c3334c0582b80d293273f2d335\src\bson\__init__.py", line 979, in decode
 [2019/11/01 15:45:11.167]     return _bson_to_dict(data, codec_options)
 [2019/11/01 15:45:11.167] bson.errors.InvalidBSON
 [2019/11/01 15:45:11.167] ======================================================================
 [2019/11/01 15:45:11.167] ERROR [0.000s]: test_encode_decode_roundtrip (test_custom_types.TestCustomPythonBSONTypeToBSONMultiplexedCodec)
 [2019/11/01 15:45:11.167] ----------------------------------------------------------------------
 [2019/11/01 15:45:11.167] Traceback (most recent call last):
 [2019/11/01 15:45:11.167]   File "C:\data\mci\da86b3c3334c0582b80d293273f2d335\src\test\test_custom_types.py", line 140, in test_encode_decode_roundtrip
 [2019/11/01 15:45:11.167]     self.roundtrip({'average': [[Decimal('56.47')]]})
 [2019/11/01 15:45:11.167]   File "C:\data\mci\da86b3c3334c0582b80d293273f2d335\src\test\test_custom_types.py", line 133, in roundtrip
 [2019/11/01 15:45:11.167]     rt_document = decode(bsonbytes, codec_options=self.codecopts)
 [2019/11/01 15:45:11.167]   File "C:\data\mci\da86b3c3334c0582b80d293273f2d335\src\bson\__init__.py", line 979, in decode
 [2019/11/01 15:45:11.167]     return _bson_to_dict(data, codec_options)
 [2019/11/01 15:45:11.167] bson.errors.InvalidBSON

And these build warnings:

 [2019/11/01 15:42:56.993] _cmessagemodule.c
 [2019/11/01 15:42:56.993] pymongo/_cmessagemodule.c(215): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.993] pymongo/_cmessagemodule.c(278): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.993] pymongo/_cmessagemodule.c(360): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.993] pymongo/_cmessagemodule.c(394): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.994] pymongo/_cmessagemodule.c(489): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.994] pymongo/_cmessagemodule.c(597): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.995] pymongo/_cmessagemodule.c(694): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.996] pymongo/_cmessagemodule.c(881): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.996] pymongo/_cmessagemodule.c(919): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.996] pymongo/_cmessagemodule.c(944): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.996] pymongo/_cmessagemodule.c(967): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.996] pymongo/_cmessagemodule.c(1057): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data
 [2019/11/01 15:42:56.999] pymongo/_cmessagemodule.c(1475): warning C4244: 'function': conversion from 'Py_ssize_t' to 'int', possible loss of data

Python 2.7 on macOS:

pymongo/_cmessagemodule.c:90:33: warning: implicit conversion loses integer precision: 'Py_ssize_t' (aka 'long') to 'int' [-Wshorten-64-to-32]
                            ns, nslen) ||       /* database */
                                ^~~~~
pymongo/_cmessagemodule.c:161:43: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
                            coll_name_len + 1)) {
                            ~~~~~~~~~~~~~~^~~
pymongo/_cmessagemodule.c:360:52: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
                            collection_name_length + 1) ||
                            ~~~~~~~~~~~~~~~~~~~~~~~^~~
pymongo/_cmessagemodule.c:489:52: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
                            collection_name_length + 1) ||
                            ~~~~~~~~~~~~~~~~~~~~~~~^~~
pymongo/_cmessagemodule.c:597:52: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
                            collection_name_length + 1) ||
                            ~~~~~~~~~~~~~~~~~~~~~~~^~~
pymongo/_cmessagemodule.c:1475:40: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
                            ns, ns_len + 1) ||  /* namespace */
                                ~~~~~~~^~~
6 warnings generated.

behackett · 2019-11-02T00:37:41Z

bson/_cbsonmodule.c

-    result = Py_BuildValue("s#", buffer_get_buffer(buffer),
-                           buffer_get_position(buffer));
-#endif
+    result = Py_BuildValue(BYTES_FORMAT_STRING, buffer_get_buffer(buffer),


I guess we don't need BYTES_FORMAT_STRING anymore. Just use y#?

We still need to use s# on Python 2.7 because y# does not exist there: https://docs.python.org/2.7/c-api/arg.html

.travis.yml

ShaneHarvey

I've fixed the warnings in the latest commit. I considered adding a test for a very large collection name but it ends up taking 10-20 seconds to run. This is the error with the C extensions:

>>> client.db['s'*(1 + 2**31)].insert_one({})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/shane/git/mongo-python-driver/pymongo/collection.py", line 698, in insert_one
    session=session),
...
  File "/Users/shane/git/mongo-python-driver/pymongo/message.py", line 706, in _op_msg
    flags, command, identifier, docs, check_keys, opts)
bson.errors.InvalidStringData: String length must be <= 2147483647

This is the error without the C extensions:

>>> client.db['s'*(1 + 2**31)].insert_one({})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/shane/git/mongo-python-driver/pymongo/collection.py", line 698, in insert_one
    session=session),
...
  File "/Users/shane/git/mongo-python-driver/bson/__init__.py", line 812, in _name_value_to_bson
    return _ENCODERS[type(value)](name, value, check_keys, opts)
  File "/Users/shane/git/mongo-python-driver/bson/__init__.py", line 622, in _encode_text
    return b"\x02" + name + _PACK_INT(len(value) + 1) + value + b"\x00"
struct.error: 'i' format requires -2147483648 <= number <= 2147483647

Maybe we should also add some Database/Collection level validation that names aren't longer than maxBsonObjectSize?

.travis.yml

ShaneHarvey · 2019-11-04T18:46:05Z

bson/_cbsonmodule.c

-    result = Py_BuildValue("s#", buffer_get_buffer(buffer),
-                           buffer_get_position(buffer));
-#endif
+    result = Py_BuildValue(BYTES_FORMAT_STRING, buffer_get_buffer(buffer),


We still need to use s# on Python 2.7 because y# does not exist there: https://docs.python.org/2.7/c-api/arg.html

ShaneHarvey · 2019-11-04T19:12:54Z

bson/_cbsonmodule.c

@@ -20,6 +20,7 @@
 * should be used to speed up BSON encoding and decoding.
 */

+#define PY_SSIZE_T_CLEAN


PEP 353 -- Using ssize_t as the index type is also worth a read. The "Conversion guidelines" section in particular. I've gone ahead and fixed the conversion warnings by using the 3 option: "otherwise, check whether the value fits an int, and raise a ValueError if it doesn't."

prashantmital

LGTM!

This change will avoid masking accidental implcit 64->32 conversions in future code that calls buffer_write_bytes.

ShaneHarvey · 2019-11-06T21:23:08Z

Fixed the failures by properly exporting _downcast_and_check according to https://docs.python.org/3/extending/extending.html?highlight=_proto#providing-a-c-api-for-an-extension-module.

prashantmital

LGTM

PYTHON-2001 Fix warnings emitted by Python 3.8

52d5ea0

Fix DeprecationWarning: PY_SSIZE_T_CLEAN will be required for '#' formats Fix DeprecationWarning: isAlive() is deprecated, use is_alive() instead Fix SyntaxWarning: invalid escape sequence Test Python 3.8 on Travis

ShaneHarvey requested a review from prashantmital November 1, 2019 21:34

ShaneHarvey commented Nov 1, 2019

View reviewed changes

Fix missing int->Py_ssize_t replacement

cf97773

Add missing Py_ssize_t conversions

c9c9764

behackett requested changes Nov 2, 2019

View reviewed changes

tacaswell mentioned this pull request Nov 2, 2019

Rebuild for python38 conda-forge/pymongo-feedstock#20

Closed

tacaswell reviewed Nov 2, 2019

View reviewed changes

.travis.yml Show resolved Hide resolved

ShaneHarvey added 2 commits November 4, 2019 10:42

Add back 3.7 travis testing

95943dc

Fix warnings by safely downcasting Py_ssize_t to int

0d85522

ShaneHarvey commented Nov 4, 2019

View reviewed changes

prashantmital approved these changes Nov 5, 2019

View reviewed changes

ShaneHarvey added 2 commits November 5, 2019 16:09

Move Py_ssize_t downcast conversion up one level

e207f0f

This change will avoid masking accidental implcit 64->32 conversions in future code that calls buffer_write_bytes.

Properly export _downcast_and_check

6c8277b

prashantmital self-requested a review November 7, 2019 21:28

prashantmital approved these changes Nov 7, 2019

View reviewed changes

ShaneHarvey merged commit 9cf0fbd into mongodb:master Nov 8, 2019

ShaneHarvey deleted the PYTHON-2001 branch November 8, 2019 19:48

ShaneHarvey mentioned this pull request May 5, 2020

add '#define PY_SSIZE_T_CLEAN' above python.h to remove deprecation warning mongodb/winkerberos#38

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

PYTHON-2001 Fix warnings emitted by Python 3.8 #428

PYTHON-2001 Fix warnings emitted by Python 3.8 #428

Uh oh!

ShaneHarvey commented Nov 1, 2019 •

edited

Loading

Uh oh!

ShaneHarvey Nov 1, 2019

Uh oh!

ShaneHarvey Nov 4, 2019

Uh oh!

ShaneHarvey commented Nov 1, 2019

Uh oh!

behackett Nov 2, 2019

Uh oh!

ShaneHarvey Nov 4, 2019

Uh oh!

Uh oh!

ShaneHarvey left a comment •

edited

Loading

Uh oh!

Uh oh!

ShaneHarvey Nov 4, 2019

Uh oh!

ShaneHarvey Nov 4, 2019

Uh oh!

prashantmital left a comment

Uh oh!

ShaneHarvey commented Nov 6, 2019

Uh oh!

prashantmital left a comment

Uh oh!

Uh oh!

PYTHON-2001 Fix warnings emitted by Python 3.8 #428

PYTHON-2001 Fix warnings emitted by Python 3.8 #428

Uh oh!

Conversation

ShaneHarvey commented Nov 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShaneHarvey Nov 1, 2019

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey Nov 4, 2019

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey commented Nov 1, 2019

Uh oh!

behackett Nov 2, 2019

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey Nov 4, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShaneHarvey left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShaneHarvey Nov 4, 2019

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey Nov 4, 2019

Choose a reason for hiding this comment

Uh oh!

prashantmital left a comment

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey commented Nov 6, 2019

Uh oh!

prashantmital left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShaneHarvey commented Nov 1, 2019 •

edited

Loading

ShaneHarvey left a comment •

edited

Loading