Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

database transactions and @transactional decorator #89

Open
JankesJanco opened this issue Nov 8, 2017 · 3 comments
Open

database transactions and @transactional decorator #89

JankesJanco opened this issue Nov 8, 2017 · 3 comments

Comments

@JankesJanco
Copy link
Member

JankesJanco commented Nov 8, 2017

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
Copy link
Member Author

Possible related issue

@JankesJanco
Copy link
Member Author

JankesJanco commented Nov 13, 2017

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:

  • sessionHolder - holds all db sessions. It would be global object or private static attribute of Service class
  • sessionProxy - proxy to current db session. This object could be hold in self._orm attribute of Service class instance

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
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant