MongoEngine / mongoengine

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

Support for MongoDB 4.0 multi-document ACID transactions #1839

Open carsonip opened 5 years ago

carsonip commented 5 years ago

Hi,

Is there any chance that MongoDB 4.0 multi-document ACID transactions feature gets supported by mongoengine? Thanks.

lonelywm commented 5 years ago

+1

shinrel22 commented 5 years ago

Still waiting for this. Hope you're working on it :(

carsonip commented 5 years ago

I'm not sure if anyone is working on this. :(

ScriptProdigy commented 5 years ago

Any news about this? Has anyone been able to do a work-around and use transactions even if mongoengine doesnt have its own interfaces for it?

lukaszkiszka commented 5 years ago

Hi @ScriptProdigy I created workaround for this:

1) From MongoEngine object I get session 2) I created Repository class where I created method for CRUD with transaction via pure pymongo code. I get pymongo Collection object from mongoengine Document. Expect added CRUD via pure pymongo I added method which they support mongoengine operations also.

See bellow code example:

import mongoengine

# model class
class TestDocument(mongoengine.Document)
    name = StringField(required=True)

# repository class code
class TestRepository:
    def __init__(self, model: Type[Document]):
        self.model = model

    # save via mongoengine
    def save(self, document: Document) -> ObjectId:
        if document:
            return document.save().id

    # transactional save via pure pymongo
    def save_with_session(self, document: Document, session): # self - type of mongoengine Document, Document - mongoengine document
        """ Transactional method """
        if document:
            document.validate() # method for validate document field - in this case validate maturity of the name field
            return self.collection.insert_one(document.to_mongo(), session=session).inserted_id

# service class code
mongo = MongoEngine()
test_repository = TestRepository(TestDocument)

# transaction save
with ctx.mongo.connection.start_session() as session:
    with session.start_transaction():
        # save definition
        test_doc = TestDocument(name='Test')
        id = test_repository.save_with_session(test_doc, session)

    # next object to transaction save
    test_doc = TestDocument(name='Test')
    id = test_repository.save_with_session(test_doc, session)

Obviusly I extract common code to BaseRepository class.

lonelywm commented 5 years ago

@lukaszkiszka nice work!!!

XZF0 commented 4 years ago

@lukaszkiszka can you please explain more on how to use this snippet? What's "MongoEngine()", ctx? How does self.collection.insert_one() work? I think I get the general intent, but this code looks incomplete, like a sketch from memory that does not compile. Thanks

XZF0 commented 4 years ago

I am not sure what is the purpose of TestRepository. Is there anything wrong with simply defining an additional method in the model class? [however, in either case the corresponding repositories need to be ensured to exist beforehand, since they cannot be created within a transaction]

class TestDocument7(mge.Document):

meta = {
    'collection': 'test_document_7',
}
name: str = StringField(required=True)  # type: ignore

def save_with_transaction(self, session):
    self.validate()
    cxn = self._get_collection()
    insert_one_result = cxn.insert_one(self.to_mongo(), session=session)
    return insert_one_result.inserted_id
florealcab commented 4 years ago

@lukaszkiszka same question as @DmitriR: what is ctx? From where do you import it?

lukaszkiszka commented 4 years ago

Hi @DmitriR and @florealcab

ctx is my copy/paste mistake - I copied code from my flask application and i forgotten delete ctx. prefix. Instead of line: with ctx.mongo.connection.start_session() as session:

Should be: with mongo.connection.start_session() as session:

@DmitriR Purpose of TestRepository is separate repository layer which working on model classes (in my example TestDocument). In my example model classes is simple classes which only represents databases entity. I built repository layer which make operations on model. It is example of three layer architecture with DAO layer.

In your case you combine together model class and business logic which works on this class - it is your architectonic decision - such entity is similar to entity in DDD aproach . I suppose your example code work well too, but queston is - you think your code is easy to manage/update in future, and it have good responsibility separation? These are rhetorical questions to draw attention to potential problems and rethink your approach.

XZF0 commented 4 years ago

Hi @lukaszkiszka - Thanks for explaining. Separation of responsibilities is a useful principle, but I am not clear how to apply it to MongoEngine-backed types specifically with respect to persistence. Multilayered designs violate the principle of simplicity and seem to obviate the main attraction of MongoEngine, which is near-transparent persistence. Could you perhaps recommend a project on GitHub that you would consider an example of well-architected, Pythonic code using MongoEngine? Or a good tutorial article perhaps. I couldn't find anything beyond unhelpfully trivial examples. MongoEngine tutorial is rudimentary. (and as a practical matter, in this case, I would probably add save_with_transaction() via a behaviors-only mixin class to all the models needing this method)

chiganov commented 4 years ago

Hi. Are there news about feature? I have spent few houers for review sourse code MongoEngine and PyMongo. It’s very sad that you cannot pass a session to the collection constructor in PyMongo (only for collection creation in mongo). So here we need to patch all code and pass session variable to all PyMongo collection methods there session variable is accepted.

Am I right that for this we have to refactor Document and QuerySet classes? I think not bad way: add variable _session to Document class, like _collection. It would be useful in contex menagers like switch_db.

Why this MR was cancelled?

I think that transactions are necessary in modern databases. Guys, we need to do something. :cry:

bagerard commented 4 years ago

Adding support for transaction requires a lot more work than simply adding support in the .save() method. This was a bit naive, we also need to make sure people are able to do any read operation in the same transaction, we also need to pay attention to the signals (pre/post_save, etc) and deletion rules (CASCADE, etc).

There is also a major decision that has to be taken in regards to the API we provide for the transaction (explicitly passing a session to all calls or trying to support it like django with a context manager and binding all operations to the session behind the scene).

I'll open another ticket to discuss this with the maintainer shortly.

chiganov commented 4 years ago

@bagerard Thank you for answer.

Nevertheless, I decided to experiment and achieved the desired result. I don’t like what I did and most likely I won’t use it in production, but it works. I created a Collection inheritance class and overwrite the methods where the session is passed. I also disabled the with_options functionality.

import mongoengine as me
import main
from app import models, usecases
from pymongo.collection import Collection

class PermanentSessionCollection(Collection):
    def __init__(self, *args, permanent_session=None, **kwargs):
        super().__init__(*args, **kwargs)
        print('permanent_session', permanent_session)
        self._permanent_session = permanent_session

    def with_options(self, codec_options=None, read_preference=None,
                     write_concern=None, read_concern=None):
        return self

    def bulk_write(self, *args, **kwargs):
        print('bulk_write')
        kwargs['session'] = self._permanent_session
        return super().bulk_write(*args, **kwargs)

    def insert_one(self, *args, **kwargs):
        print('insert_one')
        kwargs['session'] = self._permanent_session
        return super().insert_one(*args, **kwargs)

    def insert_many(self, *args, **kwargs):
        print('insert_many')
        kwargs['session'] = self._permanent_session
        return super().insert_many(*args, **kwargs)

    ... # A lot of code
    ... # where the same
    ... # thing happens

me.disconnect()
connection = me.connect('default', port=27018,  replicaset='rs0')
db = me.get_db()
session = connection.start_session()
models.Set._collection = PermanentSessionCollection(db, 
                                                    models.Set._get_collection_name(), 
                                                    permanent_session=session)
session.start_transaction()
models.Set(name='lol40').save()
models.Set.objects.get(name='lol40')
session.abort_transaction()
models.Set.objects.get(name='lol40')

The last line throws DoesNotExist exception.

I don't like it because overwriting the Сollection class is extreme and unreliable. But it works.

bagerard commented 4 years ago

fyi I've opened a discussion on the interface for supporting transaction in #2248

zhangyuz commented 3 years ago

+1

swiergot commented 1 year ago

My workaround for this problem:

def save_with_session(*args):
    with get_connection().start_session() as session:
        with session.start_transaction():
            try:
                for document in args:
                    pk = document._lookup_field("pk")[0].to_mongo(document.id)
                    modified = document._get_collection().update_one({"_id" : pk}, document._get_update_doc(), session=session).modified_count
                    if modified != 1:
                        raise Exception(f"Expected to modify 1 record of collection {document._get_collection_name()}, modified {modified}")
            except:
                session.abort_transaction()
                raise

It's so ugly and very limited but works for my use case.

RempelOliveira commented 3 months ago

Hi everyone, it's already 2024 and this seems to be a forgotten topic, any updates on this?

F4RAN commented 1 week ago

+1