MongoEngine / mongoengine

A Python Object-Document-Mapper for working with MongoDB
http://mongoengine.org
MIT License
4.24k stars 1.23k forks source link

Incompatibilities with pymongo 4.0 #2598

Closed bagerard closed 2 years ago

bagerard commented 2 years ago

There is a breaking change in pymongo 4.0 that was released very recently

It fails whenever we save an instance right now

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/local/lib/python3.8/site-packages/mongoengine/document.py:407: in save
    self.ensure_indexes()
/usr/local/lib/python3.8/site-packages/mongoengine/document.py:897: in ensure_indexes
    collection = cls._get_collection()
/usr/local/lib/python3.8/site-packages/mongoengine/document.py:220: in _get_collection
    cls.ensure_indexes()
/usr/local/lib/python3.8/site-packages/mongoengine/document.py:900: in ensure_indexes
    if not collection.is_mongos and collection.read_preference > 1:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = Collection(Database(MongoClient(host=['mongo:27017'], document_class=dict, tz_aware=False, connect=True, read_preference=Primary()), 'cpreview_test'), 'test_document.is_mongos')
    def __bool__(self):
>       raise NotImplementedError("Collection objects do not implement truth "
                                  "value testing or bool(). Please compare "
                                  "with None instead: collection is not None")
E       NotImplementedError: Collection objects do not implement truth value testing or bool(). Please compare with None instead: collection is not None
/usr/local/lib/python3.8/site-packages/pymongo/collection.py:300: NotImplementedError
bagerard commented 2 years ago

There are actually much more things that breaks with pymongo 4.0, the list of breaking changes is quite long https://pymongo.readthedocs.io/en/stable/changelog.html#breaking-changes-in-4-0

victorherbemontagne commented 2 years ago

Same problem here, we found a very bad workaround right now. (calling the function twice.... ?)

Does anybody found a better solution right now than rolling back to the previous version ?

Thanks ! :)

bagerard commented 2 years ago

This might work but it will break elsewhere for sure, I'd strongly advise you not to upgrade to pymongo 4.0 with mongoengine until this ticket gets resolved

ShaneHarvey commented 2 years ago

PyMongo maintainer here. This error actually indicates some dead-code in mongoengine. This if statement will never evaluate to True:

/usr/local/lib/python3.8/site-packages/mongoengine/document.py:900: in ensure_indexes
    if not collection.is_mongos and collection.read_preference > 1:

In PyMongo <4.0 collection.is_mongos is always True because it's a new Collection object. If "collection" is referencing a coll named "db.coll", then collection.is_mongos is a coll named "db.coll.is_mongos":

>>> collection = client.db.coll
>>> collection.full_name
'db.coll'
>>> collection.is_mongos.full_name
'db.coll.is_mongos'

Issues like this are exactly why we made the "Collection objects do not implement truth value testing or bool()" breaking change in 4.0.

collection.read_preference > 1 is also problematic since you can't compare a Collection or a read preference object.

Looking at the original issue for these lines of code I believe this check can simply be removed. PyMongo ignores the read preference for coll.create_index and always sends the command to the primary. coll.create_index is a write so read preference does not apply.

I'll follow the rest of this thread and try to help out as I can.

bagerard commented 2 years ago

Hi @ShaneHarvey , thanks a lot for helping on this.

You are absolutely right and probably the initial intent was to use [MongoClient.is_mongos] (https://pymongo.readthedocs.io/en/stable/api/pymongo/mongo_client.html#pymongo.mongo_client.MongoClient.is_mongos) instead of Collection.is_mongos but anyway, that non-working code was commited 7 years ago and is anyway not useful so we can easily throw that away.

I'll trigger a pipeline with our test suite running against pymongo 4.0, we can then use this as a starting point

bagerard commented 2 years ago

@ShaneHarvey , we still have a bunch of errors. Things that were removed but the pymongo 4.0 changelog doesn't provide information on what's the alternative (if any).

Can you advise on how to migrate these things to the new pymongo?

bagerard commented 2 years ago

Woops I ve just seen the migration guide link in the changelog , I ll get through that first 😁

victorherbemontagne commented 2 years ago

Hi @bagerard you are right, plenty stuff crash right now, do you need help on the migration ?

MuhammadAfzaal113 commented 2 years ago

I'm still getting the same error, can you guys please mention what exactly needs to change?

MuhammadAfzaal113 commented 2 years ago

I just install django-mongoengine and it solved the problem, thanks a lot.

bagerard commented 2 years ago

@victorherbemontagne Any help is welcome, feel free to branch out the current state of this PR https://github.com/MongoEngine/mongoengine/pull/2600 and help on any of the error, then open another PR. There are a lot of failures but not so much distinct errors

bagerard commented 2 years ago

@MuhammadAfzaal113 I don't see how django-mongoengine would help, it's only providing a thin wrapper to glue mongoengine and django, it's not going to magically solves the incompatibility but maybe it prevents recent pymongo version from being installed

terencehonles commented 2 years ago

Any help is welcome, feel free to branch out the current state of this PR #2600 and help on any of the error, then open another PR. There are a lot of failures but not so much distinct errors

@bagerard done in #2614

MuhammadAfzaal113 commented 2 years ago

@bagerard You're right. I explored the reason and found that the MongoDB package that I install by simply "pip install pymongo", didn't have the built-in Django-mongoengine support, so I had to install it separately.

bagerard commented 2 years ago

Just merged @terencehonles PR, if some of you could test their code against the latest master and provide a feedback, that'd be appreciated. I'll wait a few days before creating a release