long2ice / asyncmy

A fast asyncio MySQL/MariaDB driver with replication protocol support
https://github.com/long2ice/asyncmy
Apache License 2.0
230 stars 25 forks source link

Asyncmy does not get the correct host and port using "read_default_file" #74

Open pgamez opened 11 months ago

pgamez commented 11 months ago

Hi,

I've tried to start a connection with asyncmy taking the connection parameters from a config file.

This is my config file (bug.cnf):

[client]
user = myuser
host = myhost
password = mypassword
database = mydatabase

I've prepared this script to show the problem:

#!/usr/bin/env python3

import asyncio

from asyncmy import connect

async def main():
    print("# Using asyncmy directly")
    try:
        conn = await connect(
            host="myhost",
            user="myuser",
            password="mypassword",
            database="mydatabase",
        )
    except Exception as e:
        print(e)

    print("# Using asyncmy directly with a config file")
    try:
        conn = await connect(read_default_file="bug.cnf")
    except Exception as e:
        print(e)

if __name__ == "__main__":
    asyncio.run(main())

This is the output of the script:

./bug.py 
# Using asyncmy directly
(2003, "Can't connect to MySQL server on 'myhost' ([Errno -2] Name or service not known)")
# Using asyncmy directly with a config file
(1045, "Access denied for user 'myuser'@'localhost' (using password: YES)")

You can see that:

The reason of this bug is placed in the object Connection (connection.pyx). We have this function to read the data from the config file:

def _config(key, arg):
    if arg:
        return arg
    try:
        return cfg.get(read_default_group, key)
    except Exception:
        return arg

Note that the value of arg has a higher priority over the value obtained from the config file, which is requested only when arg is evaluated as False.

Then, we execute this function setting arg to the defaults:

host = _config("host", host)
port = int(_config("port", port))

but host and port defaults ('localhost' and 3306 respectively) are not evaluated to "False", so the corresponding values won't be readed from file:

def __init__(
    self,
    *,
    user=None,  # The first four arguments is based on DB-API 2.0 recommendation.
    password="",
    host='localhost',
    database=None,
    unix_socket=None,
    port=3306,
    ...
):

The solution is to set these defaults to:

In PyMySQL this bug is already fixed:

def __init__(
    self,
    ...
    host=None,
    ...
    port=0,
...
):
    ...
    self.host = host or "localhost"
    self.port = port or 3306
    ...

Thanks and greetings,

Pedro

long2ice commented 11 months ago

Thanks for report! PR welcome.

pgamez commented 11 months ago

Done! Thank you :)