Can not be configured use .commit instead .flush

[galetskiav]

Now in project after Model.create(name="zxc") i need use db.session.commit().

[aurthurm]

Override the .save() in your base class that inherits from the allfeature mixin like

def save(self):
        """Saves the updated model to the current entity db.
        """
        self.session.add(self)
        self.session.flush()
        self.session.commit()
        return self

[Shashwat986]

While this works, we will need to do the same thing for create as well, which is annoying, to say the least, and increases complexity

EDIT: It looks like create uses save internally.

[Shashwat986]

using commit() explicitly caused some issues with failed SQL queries dirtying the connection, so I've done this:

    def save(self):
        try:
            super().save()
            self.session.commit()
        except:
            self.session.rollback()
            raise

    def delete(self):
        try:
            super().delete()
            self.session.commit()
        except:
            self.session.rollback()
            raise

I'm not sure whether using except is fine or not. It works for me, but it might be a better idea to explicitly catch sqlalchemy.exc.DBAPIError

[FinnWoelm]

using commit() explicitly caused some issues with failed SQL queries dirtying the connection, so I've done this:

    def save(self):
        try:
            super().save()
            self.session.commit()
        except:
            self.session.rollback()
            raise

    def delete(self):
        try:
            super().delete()
            self.session.commit()
        except:
            self.session.rollback()
            raise

I'm not sure whether using except is fine or not. It works for me, but it might be a better idea to explicitly catch sqlalchemy.exc.DBAPIError

One small addition to the excellent save method above: We should return self at the end to mirror the save method from this package. See: https://github.com/absent1706/sqlalchemy-mixins/blob/d6dc3bdb0eb6660228d15d5482878090053ebf8d/sqlalchemy_mixins/activerecord.py#L31

Otherwise, you will not get the new instance returned when calling User.create(), for example.

[michaelbukachi]

One of the requirements for using this library is foregoing the use of explicitly commit. Do you have a specific use case as to why you need to call it explicitly?

[Shashwat986]

I think the issue is that in the Class.create(...) scenario, the DB entry is not being automatically created. Since this bug is still open, the easier solution is to overwrite the save method in the base class, so that we can continue to use the library without explicitly calling commit.

[michaelbukachi]

@Shashwat986 do you mind creating a PR. I can get this merged really quickly.

[Shashwat986]

Sure. I'll get working on it. Thanks!

[ZhymabekRoman]

Lol, I solved this problem by adding the autocommit parameter during session creation: session = scoped_session(sessionmaker(bind=engine, autocommit=True))

[deajan]

Autocommit has been deprecated with SQLAlchemy 1.4 (noted as legacy), and produces error error Engine.connect() got an unexpected keyword argument 'close_with_result' when used with session = scoped_session(sessionmaker(bind=engine, autocommit=True)) or with session = Session(engine, autocommit=True) when used with SQLModel.

Recent SQLAlchemy 1.4 (and soon to be 2.0) session syntax has a context, ie:

 Session = sessionmaker(engine)

with Session() as session:
    with session.begin():
        session.add(some_object)
    # commits

# closes the Session

I think ActiveRecordMixin should itself have the autocommit flag, like in a class variable __autocommit__ = True, or even detect what kind of session has been passed, and use contextmanager when a sessionmaker object is passed.

What do you guys think ?

I'd happily make the PR if needed.

[ethagnawl]

Hello, again, @deajan! I feel like I'm shadowing you between this project and SQLModel. grin

Did you ever come up with a clean way of doing what you'd outlined above?

[michaelbukachi]

@deajan Work has started on SQLALchemy support. The objective will be to try to maintain the same behaviour as much a the underlying library is changing cc @ethagnawl

[ethagnawl]

That's great news, @michaelbukachi! I'll follow along via that PR. I'm happy to help test when the time comes.

[michaelbukachi]

@ethagnawl You can now test #95

[ethagnawl]

I will give it a try tomorrow. Thanks, @michaelbukachi! :raised_hands:

[ethagnawl]

Arg. SQLModel isn't compatible with SQLAlchemy 2 yet. I'll have to setup a new project to test.