-
-
Notifications
You must be signed in to change notification settings - Fork 139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pass a custom pg connection object #115
Comments
These: export type DatabaseConfigurationType =
| string
| {
database?: string;
host?: string;
idleTimeoutMillis?: number;
max?: number;
password?: string;
port?: number;
user?: string;
}; are not correct types. This is the correct configuration, and you can totally use it. Lines 75 to 94 in dfd9575
Note, that it is a second parameter. slonik/src/factories/createPool.js Lines 23 to 26 in dfd9575
There is no option to pass anything but DSN as the first parameter. I am assuming you got that from definitely typed? It would be worth reporting an issue. |
Now with regards of accepting object as connection configuration and extending DSN syntax, I have already raised an issue about it. I think the proper solution would be to implement utilities I am opposed to passing a user-provided Pool instance because that highly increases the surface of the API that I need to be concerned with. We are now using only a subset of functionality provided by Pool. Allow to provide a pg Pool instance would imply that we support the entire API as we move forward, which is very unlikely. |
With regards to TLS options, depending on if you are using JavaScript driver or native driver, you would configure If you are talking about configuring a specific private key to use, then no, there is no API for that at all at the moment. It would be added to |
10x a lot for the quick response! And that would give your users a lot more familiarity with Slonik. The usual flow I've seen for the connection config is "use connection uri first, but when you encounter something unfamiliar, fall back to object", as that is a lot more explicit. That's just a suggestion though :) Anyway if you added And using cert file paths in the connection string is rather inconvenient. Those things usually come from some secrets management system that just gives you some env variables to work with, saving them to the filesystem and the passing in the filename sounds a lot more work than just passing them directly to an object. Is there anything I can do to help? |
I have a slight problem with adding additional logic to ClientUserConfigurationType. The problem with the latter is that it requires to re-compile the codebase if changes are needed. This is especially painful when working with Docker/ Kubernetes. Therefore, if anything, I am keen to remove as much as possible. I think a better approach would be to store CA/ CERT/ KEY in an environment variables. Leave both of these with me. Need to think more about the former. I will provide a solution to the latter in the next 24 hours. |
Hi, there's a use case where I think it's impossible to use a connection string. Dynamic passwords, supported by node-postgres (brianc/node-postgres#1926). Our precise use case is using RDS IAM Auth with short lived passwords. |
@gajus another reason to bump the priority of this up, many distributed tracing libraries do configuration via the connection object passed. |
Addressed in #240. |
Hi all. Is the original connection string actually addressed? I am using Cloud SQL, and also need to connect via a socket conn string. In older versions of Slonik, this worked fine. However, with the new parseDsn function, it seems to error, and no combination of fiddling with the string can i get it to work. |
To add support for socket connection you would need to update two utilities: and then (you might) need to update |
@gajus Happy to contribute the change. However, is there any desire for allowing passing in of the actual connection object, instead of a dsn? Or perhaps, allowing us to pass in the connection string itself, as it was in previous versions? I.e. allow passing in either format. It just seems a bit limiting to have to go through this dsn abstraction, as it now bottlenecks issues |
The goal of this change was to create a predictable DSN parser, because earlier version allowed to pass parameters that were quietly ignored. It will be gradually expanded to support other uses cases such as the one being discussed here. We can allow to pass an object, though it is even bigger chunk of work than the above. |
@dbousamra Did you ever get manage to connect to Cloud SQL using a socket conn string? I've wasted a day fiddling with with the connection string, came across this issue, and am about to decide to go back to Knex if it turns out there's no way to use Slonik with Cloud SQL. |
Can you please raise a separate issue identify Cloud SQL connection troubles you have? I will follow up with a solution. This issue has been largely ignored because the title misidentifies the problem. |
No I didn't. I ended up just staying on an older version of Slonik. |
You're using Postgres connection strings, which are more than sufficient for the general use-case, however configuring Slonik to play nice with more special sql offerings is a challenge.
For example connecting to an instance in Google Compute Platform requires this string
And it gets really complex because https://github.com/iceddev/pg-connection-string does not implement the full specification of https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
I also haven't found a way to pass TLS options for a socket configuration at all.
My suggestion is to just allow passing of an object that would be used directly by
pg.Pool
as opposed to passing it through https://github.com/iceddev/pg-connection-string first. I could write a PR but I work mostly with Typescript so I hesitate a bit how to implement it correctly in flow.I know you are planning on reimplementing pg.Pool yourself but it would seem prudent to keep the same config parameters. That would allow for greater backwards compatibility. I presume you would be keeping Postgres uri syntax as well.
The typescript types are a bit misleading maybe, because they state
but if you pass an object to createPool, you get an error, even leaving aside the fact that there is no TLS config in the types as well.
The text was updated successfully, but these errors were encountered: