Support for connection params in `Database` in addition to the database url

[gvbgduh]

There's a small proposal/question about the details for contenting the DB.
There might be a situation where the host might have a complex address, particularly GCP has something like:
/<cloudsql>/<project>:<region>:<db_instance>
Generally, it was solved being passed as a query param unix_socket=<host>/.s.PGSQL.5432 to the sqlalchemy engine.
It doesn't work with asyncpg as it tries to parse the db url and the : char violates the parsing logic (when such host is part of the url or passed as environment variable) or the host is empty str when it's passed as unix_socket param.

But asyncpg can take connections params separately https://magicstack.github.io/asyncpg/current/api/index.html#connection. Params have higher precedence over the url parsing, so they can take place.

I haven't come up with a great idea yet, but it might be done as

class PostgresBackend | DatabaseBackend:
    def __init__(self, database_url: typing.Union[DatabaseURL, str], params: typing.Dict) -> None:
        self._database_url = DatabaseURL(database_url)
        ...
        self._params = params

    async def connect(self) -> None:
        assert self._pool is None, "DatabaseBackend is already running"
        kwargs = self._get_connection_kwargs()
        kwargs.update(self._params)
        self._pool = await asyncpg.create_pool(str(self._database_url), **kwargs)

Would it be possible to consider?

Happy to create a PR with a reasonable solution.

[gvbgduh]

aiomysql and upcoming aiopg might also benefit from the ability of some explicit options.

[gvbgduh]

Yeah, I think I've come up with the solution that takes minimum amount of blood.
DatabaseURL already does parsing and provides DatabaseURL.options kwargs that are passed further. The only problem now that the set of those options is just {'min_size', 'max_size', 'ssl'}, so other options are ignored.

So, for example, the url 'postgresql://USER:PWD@/DB?host=HOST:project:region:db&port=5432' should do it, where in postgres it can be just extended to recognise other options
mysql might have additional backup here, like

self._pool = await aiomysql.create_pool(
    host= kwargs.pop("host", None) or self._database_url.hostname,
    ...
    autocommit=kwargs.pop("autocommit", None) or True,
    **kwargs,
)

and the mentioned url is still valid.

This gives more flexibility and control over it and extends the amount of supported urls and we still have simplicity of just the url instantiation/settings.

While I think it might be not a good idea to allow arbitrary keys in kwargs, it's worth considering to add all possible options per back end.

UPDATE: and it's still valid url for sqlalchemy engine (and hence for alembic).

Any thoughts/objections?

[lovelydinosaur]

Sounds good, yup.

Want to comment on this with how the top-level API would look.
Presumably database = Database(url, **options) right?

[gvbgduh]

Yeah, that'll do it as well 👍

But, to be honest, there's some beauty in having the db url only, however it means more complex parsing and processing logic, where it can be left for the user/framework.

And just for the record, there's an open issue for the original issue in asyncpg - MagicStack/asyncpg#419.

[gvbgduh]

that's resolved with the PR above

[coryvirok]

FYI, this PR should address the ?unix_socket= connection arg request for MySQL. #239

[keurcien]

I think there is something I am missing. I am using databases==0.4.1 to connect to a Cloud SQL instance but I didn't manage to do so. So I thought I could try to override the host using the **options, i.e:
database = Database(url, host="example").

But when I try to connect, it raises an error:
TypeError: create_pool() got multiple values for keyword argument 'host'.

Which is weird because kwargs.update(self._backend._get_connection_kwargs()) (line 72 in postgres.py) ensures that host is a unique key.

Maybe I am not doing it the right way, but I could not find anything in the documentation about connection params.

[vmarkovtsev]

@keurcien It works for me. You can try inserting a breakpoint() or import pdb; pdb.set_trace() above create_pool() and print the variables in scope to diagnose the problem. If you find an explanation, please file a separate issue. If that breakpoint does not fire or the local code mismatches, you are using an older package version, even though you think that it is ==0.4.1.

[keurcien]

Indeed the local code mismatches with the one I'm seeing in the repo. This is what I get when I run the ll command in Pdb, and presumably the one that's running on my machine and in my Docker containers, even if databases.__version__ ==0.4.1:

63         async def connect(self) -> None:
64             assert self._pool is None, "DatabaseBackend is already running"
65             kwargs = self._get_connection_kwargs()
66  ->         self._pool = await asyncpg.create_pool(
67                 host=self._database_url.hostname,
68                 port=self._database_url.port,
69                 user=self._database_url.username,
70                 password=self._database_url.password,
71                 database=self._database_url.database,
72                 **kwargs,
73             )

Should I open an issue with a minimum working app packaged in a Dockerfile so it excludes local machine problems?

N.B: I installed a version of the repo and it works fine! 👌