Database session context manager exceptions

[daviskirk]

Looking at the session context manager here:
https://github.com/onefinestay/nameko-sqlalchemy/blob/fcaef1246681f76176ca6d449c35056809d6c525/nameko_sqlalchemy/database.py#L12
I'm unsure if it is doing what I expect it to do.
I am assuming that its purpose is to allow me to do "safe" transactions knowing that if something goes wrong, it will roll back. But wouldn't most of the things that could go wrong happen during the commit? which in this case would not be covered as far as I can see.

So if I did something like:

with self.db.get_session() as session:
    session.add(new_user_with_duplicate_id_or_something_wrong)

It would not run into the rollback block (since no exception occured adding the user). Instead the exception would be thrown during the commit, it would be raised and no rollback and no close would be triggered.

I would have expected something more like this:

class Session(BaseSession):

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_val, exc_tb):
        try:
            if exc_type:
                self.rollback()
            else: 
                try:
                    self.commit()
                except Exception:
                    self.rollback()
                    raise
        finally:
            self.close()

kind of along the lines of http://docs.sqlalchemy.org/en/latest/orm/session_basics.html#when-do-i-construct-a-session-when-do-i-commit-it-and-when-do-i-close-it
(untested, just to demonstrate my thinking).
Have I misunderstood something?

[iky]

@daviskirk thanks a lot for raising the issue. You are right, the commit should be definitely included in the error handling!