Merge pull request #2837 from bagerard/remove_deprecated_warnings

remove long-warned deprecations (in prevision of 1.0)

Author: bagerard

docs/changelog.rst

@@ -12,6 +12,9 @@ Development
     - make sure to read https://www.mongodb.com/docs/manual/core/transactions-in-applications/#callback-api-vs-core-api
     - run_in_transaction context manager relies on Pymongo coreAPI, it will retry automatically in case of `UnknownTransactionCommitResult` but not `TransientTransactionError` exceptions
     - Using .count() in a transaction will always use Collection.count_document (as estimated_document_count is not supported in transactions)
+- Further to the deprecation warning, remove ability to use an unpacked list to `Queryset.aggregate(*pipeline)`, a plain list must be provided instead `Queryset.aggregate(pipeline)`, as it's closer to pymongo interface
+- Further to the deprecation warning, remove `full_response` from `QuerySet.modify` as it wasn't supported with Pymongo 3+
+- Fixed stacklevel of many warnings (to point places emitting the warning more accurately)
 
 Changes in 0.29.0
 =================

mongoengine/base/document.py

@@ -455,6 +455,7 @@ def to_json(self, *args, **kwargs):
                 "representation to use. This will be changed to "
                 "uuid_representation=UNSPECIFIED in a future release.",
                 DeprecationWarning,
+                stacklevel=2,
             )
             kwargs["json_options"] = LEGACY_JSON_OPTIONS
         return json_util.dumps(self.to_mongo(use_db_field), *args, **kwargs)
@@ -486,6 +487,7 @@ def from_json(cls, json_data, created=False, **kwargs):
                 "representation to use. This will be changed to "
                 "uuid_representation=UNSPECIFIED in a future release.",
                 DeprecationWarning,
+                stacklevel=2,
             )
             kwargs["json_options"] = LEGACY_JSON_OPTIONS
         return cls._from_son(json_util.loads(json_data, **kwargs), created=created)

mongoengine/base/metaclasses.py

@@ -307,7 +307,7 @@ def __new__(mcs, name, bases, attrs):
             and not parent_doc_cls._meta.get("abstract", True)
         ):
             msg = "Trying to set a collection on a subclass (%s)" % name
-            warnings.warn(msg, SyntaxWarning)
+            warnings.warn(msg, SyntaxWarning, stacklevel=2)
             del attrs["_meta"]["collection"]
 
         # Ensure abstract documents have abstract bases

mongoengine/connection.py

@@ -211,6 +211,7 @@ def _get_connection_settings(
             "older drivers in those languages. This will be changed to "
             "'unspecified' in a future release.",
             DeprecationWarning,
+            stacklevel=3,
         )
         kwargs["uuidRepresentation"] = "pythonLegacy"
 

mongoengine/queryset/base.py

@@ -675,7 +675,6 @@ def update_one(
     def modify(
         self,
         upsert=False,
-        full_response=False,
         remove=False,
         new=False,
         array_filters=None,
@@ -687,15 +686,7 @@ def modify(
         parameter. If no documents match the query and `upsert` is false,
         returns ``None``. If upserting and `new` is false, returns ``None``.
 
-        If the full_response parameter is ``True``, the return value will be
-        the entire response object from the server, including the 'ok' and
-        'lastErrorObject' fields, rather than just the modified document.
-        This is useful mainly because the 'lastErrorObject' document holds
-        information about the command's execution.
-
         :param upsert: insert if document doesn't exist (default ``False``)
-        :param full_response: return the entire response object from the
-            server (default ``False``, not available for PyMongo 3+)
         :param remove: remove rather than updating (default ``False``)
         :param new: return updated rather than original document
             (default ``False``)
@@ -719,9 +710,6 @@ def modify(
         sort = queryset._ordering
 
         try:
-            if full_response:
-                msg = "With PyMongo 3+, it is not possible anymore to get the full response."
-                warnings.warn(msg, DeprecationWarning)
             if remove:
                 result = queryset._collection.find_one_and_delete(
                     query, sort=sort, session=_get_session(), **self._cursor_args
@@ -746,12 +734,8 @@ def modify(
         except pymongo.errors.OperationFailure as err:
             raise OperationError("Update failed (%s)" % err)
 
-        if full_response:
-            if result["value"] is not None:
-                result["value"] = self._document._from_son(result["value"])
-        else:
-            if result is not None:
-                result = self._document._from_son(result)
+        if result is not None:
+            result = self._document._from_son(result)
 
         return result
 
@@ -1222,7 +1206,7 @@ def snapshot(self, enabled):
         :param enabled: whether or not snapshot mode is enabled
         """
         msg = "snapshot is deprecated as it has no impact when using PyMongo 3+."
-        warnings.warn(msg, DeprecationWarning)
+        warnings.warn(msg, DeprecationWarning, stacklevel=2)
         queryset = self.clone()
         queryset._snapshot = enabled
         return queryset
@@ -1331,6 +1315,7 @@ def to_json(self, *args, **kwargs):
                 "representation to use. This will be changed to "
                 "uuid_representation=UNSPECIFIED in a future release.",
                 DeprecationWarning,
+                stacklevel=2,
             )
             kwargs["json_options"] = LEGACY_JSON_OPTIONS
         return json_util.dumps(self.as_pymongo(), *args, **kwargs)
@@ -1340,24 +1325,18 @@ def from_json(self, json_data):
         son_data = json_util.loads(json_data)
         return [self._document._from_son(data) for data in son_data]
 
-    def aggregate(self, pipeline, *suppl_pipeline, **kwargs):
+    def aggregate(self, pipeline, **kwargs):
         """Perform an aggregate function based on your queryset params
 
         :param pipeline: list of aggregation commands,
             see: https://www.mongodb.com/docs/manual/core/aggregation-pipeline/
-        :param suppl_pipeline: unpacked list of pipeline (added to support deprecation of the old interface)
-            parameter will be removed shortly
         :param kwargs: (optional) kwargs dictionary to be passed to pymongo's aggregate call
             See https://pymongo.readthedocs.io/en/stable/api/pymongo/collection.html#pymongo.collection.Collection.aggregate
         """
-        using_deprecated_interface = isinstance(pipeline, dict) or bool(suppl_pipeline)
-        user_pipeline = [pipeline] if isinstance(pipeline, dict) else list(pipeline)
-
-        if using_deprecated_interface:
-            msg = "Calling .aggregate() with un unpacked list (*pipeline) is deprecated, it will soon change and will expect a list (similar to pymongo.Collection.aggregate interface), see documentation"
-            warnings.warn(msg, DeprecationWarning)
-
-        user_pipeline += suppl_pipeline
+        if not isinstance(pipeline, (tuple, list)):
+            raise TypeError(
+                f"Starting from 1.0 release pipeline must be a list/tuple, received: {type(pipeline)}"
+            )
 
         initial_pipeline = []
         if self._none or self._empty:
@@ -1380,7 +1359,7 @@ def aggregate(self, pipeline, *suppl_pipeline, **kwargs):
         if self._skip is not None:
             initial_pipeline.append({"$skip": self._skip})
 
-        final_pipeline = initial_pipeline + user_pipeline
+        final_pipeline = initial_pipeline + pipeline
 
         collection = self._collection
         if self._read_preference is not None or self._read_concern is not None:
@@ -1693,7 +1672,7 @@ def _cursor_args(self):
         # TODO: evaluate similar possibilities using modifiers
         if self._snapshot:
             msg = "The snapshot option is not anymore available with PyMongo 3+"
-            warnings.warn(msg, DeprecationWarning)
+            warnings.warn(msg, DeprecationWarning, stacklevel=3)
 
         cursor_args = {}
         if not self._timeout:

tests/queryset/test_modify.py

@@ -1,7 +1,5 @@
 import unittest
 
-import pytest
-
 from mongoengine import (
     Document,
     IntField,
@@ -32,13 +30,6 @@ def test_modify(self):
         assert old_doc.to_json() == doc.to_json()
         self._assert_db_equal([{"_id": 0, "value": 0}, {"_id": 1, "value": -1}])
 
-    def test_modify_full_response_raise_value_error_for_recent_mongo(self):
-        Doc(id=0, value=0).save()
-        Doc(id=1, value=1).save()
-
-        with pytest.raises(ValueError):
-            Doc.objects(id=1).modify(set__value=-1, full_response=True)
-
     def test_modify_with_new(self):
         Doc(id=0, value=0).save()
         doc = Doc(id=1, value=1).save()

tests/queryset/test_queryset.py

@@ -474,8 +474,7 @@ class A(Document):
 
         A.drop_collection()
 
-        for i in range(100):
-            A.objects.create(s=str(i))
+        A.objects.insert([A(s=str(i)) for i in range(100)], load_bulk=True)
 
         # test iterating over the result set
         cnt = 0
@@ -1283,8 +1282,7 @@ def __repr__(self):
 
         Doc.drop_collection()
 
-        for i in range(1000):
-            Doc(number=i).save()
+        Doc.objects.insert([Doc(number=i) for i in range(1000)], load_bulk=True)
 
         docs = Doc.objects.order_by("number")
 
@@ -5438,8 +5436,9 @@ class Person(Document):
             name = StringField()
 
         Person.drop_collection()
-        for i in range(100):
-            Person(name="No: %s" % i).save()
+
+        persons = [Person(name="No: %s" % i) for i in range(100)]
+        Person.objects.insert(persons, load_bulk=True)
 
         with query_counter() as q:
             assert q == 0
@@ -5469,8 +5468,9 @@ class Person(Document):
             name = StringField()
 
         Person.drop_collection()
-        for i in range(100):
-            Person(name="No: %s" % i).save()
+
+        persons = [Person(name="No: %s" % i) for i in range(100)]
+        Person.objects.insert(persons, load_bulk=True)
 
         with query_counter() as q:
             assert q == 0
@@ -5537,17 +5537,20 @@ def __unicode__(self):
         assert 1 == len(users._result_cache)
 
     def test_no_cache(self):
-        """Ensure you can add meta data to file"""
+        """Ensure you can add metadata to file"""
 
         class Noddy(Document):
             fields = DictField()
 
         Noddy.drop_collection()
+
+        noddies = []
         for i in range(100):
             noddy = Noddy()
             for j in range(20):
                 noddy.fields["key" + str(j)] = "value " + str(j)
-            noddy.save()
+            noddies.append(noddy)
+        Noddy.objects.insert(noddies, load_bulk=True)
 
         docs = Noddy.objects.no_cache()
 
@@ -5766,8 +5769,9 @@ class Person(Document):
             name = StringField()
 
         Person.drop_collection()
-        for i in range(100):
-            Person(name="No: %s" % i).save()
+
+        persons = [Person(name="No: %s" % i) for i in range(100)]
+        Person.objects.insert(persons, load_bulk=True)
 
         with query_counter() as q:
             if Person.objects:

tests/queryset/test_queryset_aggregation.py

@@ -1,6 +1,6 @@
 import unittest
-import warnings
 
+import pytest
 from pymongo.read_preferences import ReadPreference
 
 from mongoengine import *
@@ -216,7 +216,7 @@ class Person(Document):
 
         assert list(data) == [{"_id": p3.pk, "name": "SANDRA MARA"}]
 
-    def test_queryset_aggregation_deprecated_interface(self):
+    def test_queryset_aggregation_old_interface_not_working(self):
         class Person(Document):
             name = StringField()
 
@@ -227,27 +227,20 @@ class Person(Document):
         p3 = Person(name="Sandra Mara")
         Person.objects.insert([p1, p2, p3])
 
-        pipeline = [{"$project": {"name": {"$toUpper": "$name"}}}]
-
-        # Make sure a warning is emitted
-        with warnings.catch_warnings():
-            warnings.simplefilter("error", DeprecationWarning)
-            with self.assertRaises(DeprecationWarning):
-                Person.objects.order_by("name").limit(2).aggregate(*pipeline)
+        _1_step_pipeline = [{"$project": {"name": {"$toUpper": "$name"}}}]
 
-        # Make sure old interface works as expected with a 1-step pipeline
-        data = Person.objects.order_by("name").limit(2).aggregate(*pipeline)
+        # Make sure old interface raises an error as we changed it >= 1.0
+        with pytest.raises(TypeError, match="pipeline must be a list/tuple"):
+            Person.objects.order_by("name").limit(2).aggregate(*_1_step_pipeline)
 
-        assert list(data) == [
-            {"_id": p1.pk, "name": "ISABELLA LUANNA"},
-            {"_id": p3.pk, "name": "SANDRA MARA"},
+        _2_step_pipeline = [
+            {"$project": {"name": {"$toUpper": "$name"}}},
+            {"$limit": 1},
         ]
-
-        # Make sure old interface works as expected with a 2-steps pipeline
-        pipeline = [{"$project": {"name": {"$toUpper": "$name"}}}, {"$limit": 1}]
-        data = Person.objects.order_by("name").limit(2).aggregate(*pipeline)
-
-        assert list(data) == [{"_id": p1.pk, "name": "ISABELLA LUANNA"}]
+        with pytest.raises(
+            TypeError, match="takes 2 positional arguments but 3 were given"
+        ):
+            Person.objects.order_by("name").limit(2).aggregate(*_2_step_pipeline)
 
     def test_queryset_aggregation_geonear_aggregation_on_pointfield(self):
         """test ensures that $geonear can be used as a 1-stage pipeline and that
@@ -272,7 +265,7 @@ class Aggr(Document):
                 }
             }
         ]
-        assert list(Aggr.objects.aggregate(*pipeline)) == [
+        assert list(Aggr.objects.aggregate(pipeline)) == [
             {"_id": agg1.id, "c": 0.0, "name": "X"},
             {"_id": agg2.id, "c": 0.0, "name": "Y"},
         ]