Allow configuration options for Postgres #2873
Conversation
|
@kulshekhar updated the pull request - view changes |
|
@kulshekhar can you please add unit tests for the configuration parsing please? so we make sure this piece is correctly covered? Maybe refactor into a PGConfiguration parser (that would parse the options) and then keep the initialization 'outside' the configuration parser? |
|
Sure. Just so we're on the same page, would this mean moving the newly created |
|
Yep That would be nice, so that's properly tested on itself with different URLs and options, and properly covered. |
|
@kulshekhar updated the pull request - view changes |
| function getDatabaseOptionsFromURI(uri) { | ||
| const databaseOptions = {}; | ||
|
|
||
| try { |
There was a problem hiding this comment.
Why do we surround with try/catch just to retrhow?
There was a problem hiding this comment.
Forgot to remove that. Will remove.
| module.exports = { | ||
| parseQueryParams: parseQueryParams, | ||
| getDatabaseOptionsFromURI: getDatabaseOptionsFromURI | ||
| }; |
There was a problem hiding this comment.
Add line at the end of the file
| @@ -0,0 +1,31 @@ | |||
| const pgp = require('pg-promise')(); | |||
There was a problem hiding this comment.
Maybe rename file to PGClient
There was a problem hiding this comment.
Or PostgresClient for the sake of consistency :)
There was a problem hiding this comment.
This isn't the client. It's just pg-promise being initialized. A call to pgp() is what creates the client.
There was a problem hiding this comment.
So PostgresClient.createClient() still makes sense :)
There was a problem hiding this comment.
I had misread your comment. I didn't realize you had asked for the file name to be changed (The code snippet made me think you'd asked for the variable name to be changed!)
I've updated the file name to PostgresClient
|
|
||
| databaseOptions.client_encoding = queryParams.client_encoding; | ||
| databaseOptions.application_name = queryParams.application_name; | ||
| databaseOptions.fallback_application_name = queryParams.fallback_application_name; |
There was a problem hiding this comment.
Are there additional options possible?
There was a problem hiding this comment.
These are the only options mentioned here: https://github.com/vitaly-t/pg-promise/wiki/Connection-Syntax#configuration-object
There was a problem hiding this comment.
Ok good, we'll keep an eye if new options arise from that module
|
@kulshekhar updated the pull request - view changes |
|
@kulshekhar updated the pull request - view changes |
|
@kulshekhar updated the pull request - view changes |
|
@kulshekhar updated the pull request - view changes |
|
nice fuction! |
|
name: 'error', |
Resolves #2872