2
\$\begingroup\$

I have written this code that connects to a mysql database. The function should be able to be used as a context manager - via with. It is working, but I am not sure if this is a great way of doing it.

conect.py

import os
import mysql.connector
from mysql.connector import errorcode
import contextlib

_CFG = {
  'user': os.getenv("MYSQL_USER"),
  'password': os.getenv("MYSQL_PASSWORD"),
  'host': os.getenv("MYSQL_SERVER"),
  'database': os.getenv("SENSOR_DB"),
  'raise_on_warnings': True
}

@contextlib.contextmanager
def _connect(db=None):
    if db:
        _CFG["database"] = db

    try:
        print("{usr} connecting to {db} on {host}.".format(
            usr=_CFG["user"],
            db=_CFG["database"],
            host=_CFG["host"]
            )
        )
        connection = mysql.connector.connect(**_CFG)
        yield connection
    except mysql.connector.Error as err:
        if err.errno == errorcode.ER_ACCESS_DENIED_ERROR:
            print("Something is wrong with your user name or password")
        elif err.errno == errorcode.ER_BAD_DB_ERROR:
            print("Database does not exist")
        else:
            print(err)
        yield None
    else:
        connection.commit()
    finally:
        connection.close()
\$\endgroup\$
2
\$\begingroup\$
  1. Obvious (dumb) point - I'd suggest using logging instead of print
  2. I would have some kind of builder for _CFG so I could choose if I use environment variables for configuration or something else
  3. Instead of str.format, you may want to use fstring: f"{_CFG["user"]} connecting to {_CFG["database"]} on {_CFG["host"]}."
  4. Instead of yield None you may want to reraise exception (from the docs, "If an exception is trapped merely in order to log it or to perform some action (rather than to suppress it entirely), the generator must reraise that exception"). Otherwise, your code will fail with a funny exception.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I was wondering even if the _connect function acts as context manger , should I just call it at module level instead of using with inside a function, then the connection object will be available at multiple functions ? \$\endgroup\$ – Ciasto piekarz Jan 26 at 18:43
  • \$\begingroup\$ @Ciastopiekarz I think that if you want to have a shared connection in multiple functions, you would rather not use a context manager at all, but some other patterns to handle this. But why would you want this? I would not bother unless I am sure there's some significant performance impact, but I don't think there would be. \$\endgroup\$ – Alexander Pushkarev Jan 27 at 10:25
  • \$\begingroup\$ Ok, cool thanks I tried yesterday and didn’t find any significant benefit so switched back. \$\endgroup\$ – Ciasto piekarz Jan 27 at 10:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.