MagicStack / asyncpg

A fast PostgreSQL Database Client Library for Python/asyncio.
Apache License 2.0
6.99k stars 404 forks source link

Connection methods does not accept classic nor unusual arguments supported in other pgsql connection solution #979

Open titouanfreville opened 1 year ago

titouanfreville commented 1 year ago

Hello,

While trying to configure a Cockroach DB cluster async connection, there seems to be missing support for usual PostgreSQL parameters (or renamed) or supported options not frequently used.

I don't know why the connection part does not seems to follow other library but it would be nice to at least support all options existing in postgresql client (even renamed ^^).

For example, the part I am missing to connect is options keyword. It is used to provide complementary informations on the connection (in my case a cluster id to identify exact connection). Guess it is not mandatory by itself but it fells strange to have the issue only on asyncpg :/

elprans commented 1 year ago

asyncpg is largely compatible with libpq conventions. Can you be more specific?

or example, the part I am missing to connect is options keyword.

asyncpg calls this server_settings

titouanfreville commented 1 year ago

Hello and thanks for the quick reply.

Then I guess the true issue here is a rename between usual connection string parameters and async connect parameters naming.

I guessed it followed a lib spec, but did not found anything on options so though it was absent. Still feels strange that some options are renamed in the interface (ssl/options) are there others remaps ?

Also, it does not seems documented, coming from SQLAlch, I do not have direct control on the parameters named and have to remap them in my connection sting so they are passed correctly to connection instance. While taking a look on this, I also though it would have been more user friendly to have a dual map in renamed fields if possible so coming from other ecosystem do not require hours of trials and errors :)

elprans commented 1 year ago

Also, it does not seems documented, coming from SQLAlch, I do not have direct control on the parameters named and have to remap them in my connection sting so they are passed correctly to connection instance.

Well, this isn't an asyncpg issue then. If things aren't clear or don't work, raise an issue on the SQLAlchemy tracker.

titouanfreville commented 1 year ago

Yes, I'll also oppen an issue on their side to remap parameters :)

Still, I tried with the server_settings keywords instead of options and it is still not working. server_settings expect a dict, so I tired to provide both {"--cluster": "XXXX"} and {"cluster": "XXXX"} and none worked. Is the mapping correct ?

elprans commented 1 year ago

server_settings={"cluster": "XXXX"} should work. If it doesn't I'd ask on the CockroachDB tracker.

titouanfreville commented 1 year ago

Thanks for the help. I will retry with this and let you know if I manage to make it run :)

titouanfreville commented 1 year ago

So I confirm that cluster is not passed through (don't know why).

asyncpg.exceptions.ConnectionRejectionError: codeParamsRoutingFailed: missing cluster identifier

 ConnectionParameters(user='postgres', password='XXXXXXXXX', database='XXXXXXX', ssl=<ssl.SSLContext object at 0x7f624ce21040>, sslmode=<SSLMode.verify_full: 5>, direct_tls=False, connect_timeout=60, server_settings=None) ('free-XXXX.gcp-europe-west1.cockroachlabs.cloud', 26257) ConnectionConfiguration(command_timeout=None, statement_cache_size=100, max_cached_statement_lifetime=300, max_cacheable_statement_size=15360) ConnectionParameters(user='postgres', password='XXXXXXX', database='betonyou', ssl=<ssl.SSLContext object at 0x7f624ce21040>, sslmode=<SSLMode.verify_full: 5>, direct_tls=False, connect_timeout=60, server_settings={'cluster': 'test-boy-4144'})

But also, it is not an issue anymore as Cockroach change their system to use a dedicated URL instead of cluster option ^^

Sorry for the time lost and thanks again for the help. I'll open issues on SQLAlchemy sides for the parameters conversion :)