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

Transaction Events #29

Open
gergamel opened this issue Nov 7, 2018 · 3 comments
Open

Transaction Events #29

gergamel opened this issue Nov 7, 2018 · 3 comments

Comments

@gergamel
Copy link

gergamel commented Nov 7, 2018

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
Copy link
Owner

dgilland commented Nov 7, 2018

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

gergamel commented Nov 9, 2018

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
Copy link
Owner

dgilland commented Nov 9, 2018

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.

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

2 participants