dwyl / hapi-postgres-connection

:zap: Creates a PostgreSQL Connection (Pool) available anywhere in your Hapi App
GNU General Public License v2.0
40 stars 13 forks source link

Provide some way to enable pg options? #14

Closed jacobtipp closed 1 year ago

jacobtipp commented 8 years ago

I'm using the postgres provided by Heroku. I was going to use this module but was getting Error connection issues. Apparently Heroku requires ssl defaults set to true

const pg = require('pg')

pg.defaults.ssl = true

pg.connect(process.env.DATABASE_URL, (err, client, done) => {......

The workaround for this is to either require the pg module separately and apply this setting at the root of my server file. While this works I'd rather not install a dependency that is already required by this module. Perhaps it may be better to add a way for this plugin to handle these configuration options.

nelsonic commented 8 years ago

Plugin options would be a good idea. Are there any others besides SSL you need for the first version?

jacobtipp commented 8 years ago

@nelsonic, I only need the ssl for mine to work, although, I do think it should support all of the pg.defaults properties described in the documentation.

https://github.com/brianc/node-postgres/wiki/pg

nelsonic commented 8 years ago

@traducer agree that we should support the pg.defaults https://github.com/brianc/node-postgres/wiki/pg#pgdefaults should we do a 1:1 mapping or simply an Object.keys lookup?

jacobtipp commented 8 years ago

@nelsonic , I think something like this should work

options.defaults = {...} //user provides their defaults

/* creating pool */

var pg = require('pg')

for(var i = 0, keys = Object.keys(options.defaults), len = keys.length; i < len; ++i) {
  /* mapping happens here */
    pg.defaults[keys[i]] = options.defaults[keys[i]]
}

The reason I think defaults should be supported is because some of them are meant to be assigned before the pool is created. The way Hapi-postgres-connection was built looks like the pool is created in a closure before you register the plugin. I'm new to Hapi so I don't want to make too many assumptions heh. I just wouldn't know how to expose those options to the single pool without wrapping that pool in some kind of initialize function and exporting it with the plugin.

nelsonic commented 8 years ago

@traducer that looks like a good option. :+1: given that the setting of options will only be run once at server.start there's no perf hit for writing:

Object.keys(pg.defaults).forEach(function (k) {
  pg.defaults[k] = options.defaults[k] || pg.defaults[k];
});

Yes, you are 100% correct, the current implementation of the plugin is connecting to PostgreSQL before the Hapi server starts: index.js#L11 We can change that to wrap the pg.connect in a function that accepts the options object, we just need to ensure that our existing tests pass, because this plugin is being used in a project where tests were choking because of the pg connection lead time...

jacobtipp commented 8 years ago

Ah gotcha, either way this is a learning experience for me. I haven't used much of postgres and this plugin made it a lot easier to have access to PG and simplified pooling in my routes.

nelsonic commented 8 years ago

Yeah, our objective was to simplify using PostgreSQL in our Hapi Handlers. We achieved that. Now, if we want to make the plugin/module useful to other people, adding the options seems like a good place to start. 👍