Stop two sources of dangling transactions #2381

Merged
merged 4 commits into from Jul 21, 2015

Projects

None yet

2 participants

@tilgovi
Contributor
tilgovi commented Jul 20, 2015

Two sources of dangling transactions:

  • WebSocket

Solved by releasing the transaction after checking features,
avoiding a transaction to get the user for every 'more_hits' call,
and the general hygiene of processing messages in a transaction.

  • Notification worker

Process messages in a transaction.

@nickstenning nickstenning commented on an outdated diff Jul 21, 2015
h/streamer.py
@@ -533,10 +537,7 @@ def received_message(self, msg):
self.client_id = data.get('value')
except:
log.exception("Parsing filter: %s", msg)
- transaction.abort()
@nickstenning
nickstenning Jul 21, 2015 Member

This pathway previously aborted the transaction, and now doesn't. Reraise?

@tilgovi
Contributor
tilgovi commented Jul 21, 2015

@nickstenning good to go, thanks for the review

tilgovi added some commits Jul 20, 2015
@tilgovi tilgovi Stop two sources of dangling transactions
Two sources of dangling transactions:

- WebSocket

Solved by releasing the transaction after checking features,
avoiding a transaction to get the user for every 'more_hits' call,
and the general hygiene of processing messages in a transaction.

- Notification worker

Process messages in a transaction.
f994ad9
@tilgovi tilgovi Reraise on exception in streamer message 7b43e7c
@tilgovi tilgovi Add cleanup TODO note
560aa5e
@tilgovi tilgovi Don't choke while I plot your demise
I will see you again, soon, streamer.py. Soon.
472a385
@tilgovi
Contributor
tilgovi commented Jul 21, 2015

There, that should get the tests passing.

@nickstenning
Member

WAT?

@tilgovi
Contributor
tilgovi commented Jul 21, 2015

Should I do anything else here?

@nickstenning nickstenning merged commit e0c868b into master Jul 21, 2015

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 60.838%
Details
@nickstenning nickstenning deleted the cleanup-transactions branch Jul 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment