MolSSI / QCFractal

A distributed compute and database platform for quantum chemistry.
https://molssi.github.io/QCFractal/
BSD 3-Clause "New" or "Revised" License
144 stars 47 forks source link

[next branch] `urllib.parse.urlparse(full_uri)` fails with Unix socket #727

Closed hadim closed 11 months ago

hadim commented 1 year ago

Since the parsing fails, the pydantic config becomes invalid.

Details below:

# https://cloud.google.com/sql/docs/mysql/connect-auth-proxy#connect-unix

import urllib

# Using a cloudsql URIm the parsing fails and so the QCF config is invalid
full_uri = "postgres://username:password@//cloudsql/project:region:instance-id/db_name"
parsed = urllib.parse.urlparse(full_uri)
parsed.hostname, parsed.port, parsed.username, parsed.password, parsed.query
# Outputs: (None, None, 'username', 'password', '')

# Using a standard URI, everything works fine
full_uri = "postgres://username:password@127.0.0.1:777/db_name"
parsed = urllib.parse.urlparse(full_uri)
parsed.hostname, parsed.port, parsed.username, parsed.password, parsed.query
# Outputs: ('127.0.0.1', 777, 'username', 'password', '')
hadim commented 1 year ago

For now, I simply disabled all the URI parsing logics at https://github.com/hadim/QCFractal/commits/next_hadim/qcfractal (check latest commits)

And it works fine.


Note that I also had other issues but probably more related to Cloud Run and SQL than QCF. I also had to change the URI format to db_full_uri = "postgresql://${google_sql_user.db_user.name}:${google_sql_user.db_user.password}@/${local.database_name}?host=/cloudsql/${google_sql_database_instance.db_instance.connection_name}"

hadim commented 1 year ago

@bennybp would you consider supporting full_uri without doing too much parsing on it?

I have two commits (likely not ideal) that make it work with the above sock-based URI:

bennybp commented 1 year ago

Yes I am absolutely interesting in supporting that (and was one of the reasons I started putting it in. I want to be able to run via socket as well).

Let me get to this in the next few days

bennybp commented 1 year ago

It took a few iterations, but the changes in https://github.com/MolSSI/QCFractal/commit/1be6db89a3e78460f67207147725c77315eaa72a should fix this.

It's probably easiest to put the host in the query string (like you had before I think. See https://github.com/MolSSI/QCFractal/blob/1be6db89a3e78460f67207147725c77315eaa72a/qcfractal/qcfractal/test_db_connection.py#L212)

bennybp commented 11 months ago

See previous comment. Everything should be working with unix sockets now.