ocf / ocflib

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

consolidate get_connection across ocflib #103

Closed abizer closed 6 years ago

chriskuehl commented 6 years ago

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 6 years ago

Oh lol, I thought we closed them again, but it makes some sense we don't since we just return a connection that is never actually closed anywhere. That does sound like a good idea to have something to close the connections when they are done.

abizer commented 6 years ago

I'll put it on my todo list to start upgrading the API across the codebase.

jvperrin commented 6 years ago

Alright, I've opened an issue in #108, since I think that's a bit of a separate issue, but definitely one that should be addressed sometime.