search

ocf / ocflib

Python libraries for account and server management
https://pypi.python.org/pypi/ocflib
Other
15 stars 32 forks source link

Improve database connection API to actually close opened connections #108

Open jvperrin opened 6 years ago

jvperrin commented 6 years ago

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 conn

Then 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.

jvperrin commented 5 years ago

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.

dkess commented 5 years ago

I'm confused on this. Was the with syntax deprecated? If so, if we never implemented it, what do we need to do?

jvperrin commented 4 years ago

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)