AtteqCom / zsl

zsl application framework for web based services
MIT License
1 stars 1 forks source link

database transactions and @transactional decorator #89

Open JankesJanco opened 6 years ago

JankesJanco commented 6 years ago

Call inside @transactional method to another @transactional method from other Service class instance lead in some cases to error.

Following code will result in exception (sqlalchemy.orm.exc.DetachedInstanceError) raised at line foo.bar_parent_name = bar.parent.name when calling method FooService.create_foo()

class FooService(Service):

    @inject(bar_service=BarService)
    def __init__(self, bar_service: BarService):
        self._bar_service = bar_service

    @transactional
    def create_foo(self):
        foo = Foo()
        bar = self._bar_service.get_bar()

        foo.bar_parent_name = bar.parent.name

        # ...

class BarService(Service):

    def get_bar(self):
        self._orm.query(Bar).first()

If you want, I can provide real-life use-case similar to this example. This behavior limit options to better split code into logic and reusable units.

Maybe we should open discussion about @transactional decorator design and implementation. Briefly said, nowadays @transactional decorator set db session to _orm property of Service class instance. I am not sure if it is ideal design. Maybe we could get inspiration from Java Spring framework where 2 types of classes are used - Services and Daos - and db session is not directly bound to class instances as in our case.

JankesJanco commented 6 years ago

Possible related issue

JankesJanco commented 6 years ago

Definition: I will use term transactional method for method which is decorated by @transactional decorator

Design goals

  1. ability to call inside transactional method other methods (from another classes) which also work with db session. Good example is architecture with dao services and business services where transaction is initiated on business service method and real querying to db is made only in dao service methods (which are called from business service methods).
  2. concurrency compatible solution
  3. maybe coroutines compatible solution
  4. maybe support for transaction propagation modes (but this is not crucial for now). Something like this in Spring framework

Design proposals:

1. Session as parameter in transactional method

class MyGreatService(Service):

    @transactional
    def get_foo(self, foo_id, db_session):
        # ...
        db_session.query(Foo).get(foo_id)
        # ...

# usage

# db_session argument is set by @transactional decorator
my_great_service.get_foo(3)

# or when we already are inside transactional method, we can use existing db session
my_great_service.get_foo(3, db_session=existing_db_session)

Question: Can we get rid of Service class (base class of MyGreatService class) in case of this design?

Pros:

  1. achieves all 4 goals

Cons:

  1. not so comfortable to use as in case of second proposal
  2. not backward compatible Note 1: There was proposal to keep self._orm attribute from current design (for backward compatibility). I think, it is not good idea because it breaks second condition from design goals. Note 2: I think we can sacrifice backward compatibility.

2. Sessions stored in sessionHolder object, current session accessible through sessionProxy

There are 2 crucial objects in this design:

Note: For implementation we can use Local, LocalStack and LocalProxy classes from werkzeug.

Pros:

  1. easy to use
  2. backward compatible

Cons:

  1. Third condition from design goals is not achieved (Am I right?)
  2. Usage of global object - sessionHolder
JankesJanco commented 6 years ago

After discussion in telegram the winning proposal is the first one Session as parameter in transactional method. The main reason is that first proposal meets all 4 design goals and is somehow cleaner solution while cons of second proposal (Sessions stored in sessionHolder object, current session accessible through sessionProxy) are significant.