Skip to content

Commit

Permalink
Log a warning if a Pyramid request ends with uncommitted DB session c…
Browse files Browse the repository at this point in the history
…hanges

The previous `session.dirty` check only tested for _changes_ to existing
DB objects which have not been flushed. It did not check for added or
deleted model objects which had not yet been flushed to the DB, or for
changes which have been flushed inside a transaction but not yet _committed_.

When debugging #4704 we want to
specifically catch additions (of annotations) to the DB which have been
flushed (by `storage.create_annotation`) but not yet committed.
  • Loading branch information
robertknight committed Jan 8, 2018
1 parent e13b9ab commit 0d88f15
Showing 1 changed file with 12 additions and 3 deletions.
15 changes: 12 additions & 3 deletions h/db/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import sessionmaker

from h.util.session_tracker import Tracker

__all__ = (
'Base',
'Session',
Expand Down Expand Up @@ -82,6 +84,10 @@ def _session(request):
else:
zope.sqlalchemy.register(session, transaction_manager=tm)

# Track uncommitted changes so we can verify that everything was either
# committed or rolled back when the request finishes.
tracker = Tracker(session)

# pyramid_tm doesn't always close the database session for us.
#
# For example if an exception view accesses the session and causes a new
Expand All @@ -96,10 +102,13 @@ def _session(request):
# See: https://github.com/Pylons/pyramid_tm/issues/40
@request.add_finished_callback
def close_the_sqlalchemy_session(request):
if session.dirty:
request.sentry.captureMessage('closing a dirty session', stack=True, extra={
'dirty': session.dirty,
changes = tracker.uncommitted_changes()
if changes:
msg = 'closing a session with uncommitted changes'
request.sentry.captureMessage(msg, stack=True, extra={
'changes': changes,
})
log.warn('{} {}'.format(msg, changes))
session.close()

return session
Expand Down

0 comments on commit 0d88f15

Please sign in to comment.