dgilland / sqlservice

The missing SQLAlchemy ORM interface.
https://sqlservice.readthedocs.io
MIT License
179 stars 9 forks source link

Transaction Events #29

Open gergamel opened 6 years ago

gergamel commented 6 years ago

Hi Derrick,

I was using SQLAlchemy session events to run some code on the "before_commit" event. For example:

def pre_commit(session):
    print("Session pre_commit() event")

engine = create_engine(db_uri)
session = scoped_session(sessionmaker(autocommit=False,autoflush=False,bind=engine))
event.listen(session, "before_commit", pre_commit)

If you try and use an SqlClient instance as the first argument to even.listen(), you get:

  File ".../lib/python3.6/site-packages/sqlalchemy/event/api.py", line 28, in _event_key
    (identifier, target))
sqlalchemy.exc.InvalidRequestError: No such event 'before_commit' for target '<class 'sqlservice.client.SQLClient'>'

I managed to get around this by using the _Session. Working code follows:

def pre_commit(session):
    print("Session pre_commit() event")

class MyClient(SQLClient):
    def __init__(self, echo=False):
        self.config = get_default_config()
        super().__init__(self.config, model_class=Base)
        event.listen(self._Session, "before_commit", pre_commit)

It took a little time to figure out the above because your docs are specific to ORM events. I can see a couple of options to help others in the future but wanted to get your thoughts before taking it further:

  1. Update docs/event.rst to show how it can be done (see below)
  2. Wrap sqlservice.event (probably in src/sqlservice/event.py and handle special cases like this directly
dgilland commented 6 years ago

Thanks for reporting!

This is definitely something that could be improved. Updating the docs would help but beyond just having an example of event.listen(db._Session, ...) or event.listen(db.engine, ...), an api could be added to SQLClient to make this easier.

Something like:

db = SQLClient()
db.subscribe_session('before_commit', callback)
db.subscribe_engine('connect', callback)

# or maybe just db.subscribe() that automaps event name to session or engine
db.subscribe(...)

Thoughts?

gergamel commented 6 years ago

I really like the way you wrapped SA's event API so the event callbacks can live in the model definition, keeping everything together (as per the example here).

Keeping this style, but without requiring devs to extend the SQLClient() class, what do you think about something like:

db = SQLClient()

@db.listen('before_commit')
def pre_commit(client):
  print("Session 'before_commit' event")

#or maybe, explicit decorator per event name, rather than SA's string arg approach?
@db.before_commit
def pre_commit(client):
  print("Session 'before_commit' event")
dgilland commented 6 years ago

I like where you're going with that but with a small tweak to mirror sqlalchemy.event API:

@db.listens_for('before_commit')
def pre_commit(...)

db.listen('before_commit', pre_commit)

which would match sqlalchemy.event.listen and sqlalchemy.event.listens_for.

As for the named event decorators, those could be useful too but might potentially namespace them to db.event. There are some events that might not be as obvious (e.g. if supporting pool or connection events, there are ones like "close" and "checkin|out"). Although, could just prefix them with "on_" like in sqlservice.event. Another option might be to have standalone decorators similar to sqlservice.event but those would require passing the db instance like @before_commit(db).

But as a first iteration, I think just having SQLClient.listen and SQLClient.listens_for would be sufficient with a more expansive integration coming later.