ocflib
ocflib copied to clipboard
Improve database connection API to actually close opened connections
Taken from @chriskuehl's comment on #103:
This is fine, but eventually we should take the opportunity to fix this API. Currently it's really hard to write correct code which closes connections as you'd expect; a better API would be something like this:
@contextlib.contextmanager def get_connection(...): with contextlib.closing(pymysql.connect(...)) as conn: yield connThen usage is like this:
with get_connection() as conn: with conn as cursor: cursor.execute('...')Currently we never call close on the connections we open.
This appears to have been officially deprecated now in pymysql (https://github.com/PyMySQL/PyMySQL/commit/1ef6c587337bd6ff3272c2c4771948676fd2a9e6) due to https://github.com/PyMySQL/PyMySQL/issues/735, so we should get to this fix in the next year, preferably sooner.
I'm confused on this. Was the with syntax deprecated? If so, if we never implemented it, what do we need to do?
We're getting deprecation notices about this in test run outputs of ocflib now:
.tox/py35/lib/python3.5/site-packages/pymysql/connections.py:497
[repeats many more times]
.tox/py35/lib/python3.5/site-packages/pymysql/connections.py:497
/opt/jenkins/slave/workspace/ocflib_PR-201/.tox/py35/lib/python3.5/site-packages/pymysql/connections.py:497: DeprecationWarning: Context manager API of Connection object is deprecated; Use conn.begin()
As for the most recent question on this, I realize I didn't actually respond to it. I think https://stackoverflow.com/a/31215864 does a much better job answering the core issue, so I'll link to that one. While we didn't implement our own version of it or anything, we do use connection objects with with in quite a few places: https://sourcegraph.ocf.berkeley.edu/search?q=with+.*+conn (this probably has a few false positives, but close enough)