Skip to content

Conversation

@slickmb
Copy link

@slickmb slickmb commented Sep 12, 2014

Ran into a real-world use-case in node-postgres where the host name was being set using an environment variable, but we needed to change the database in code. node-postgres was being wrapped in other code that we could not edit for beurocratic reasons. This code prevented us from passing config objects (i.e. forced us to pass a connection 'string'). The current code assumes that the pathname returned by nodejs will always have a leading '/'. This is true for fully qualified urls, but NOT true for relative urls. This caused pg-connection-string to return a database config object where the database was missing it's first character. There are work-arounds, but it seems like something that is easily supported.

var pg = require('pg');

console.log(process.env.PGHOST); // '/tmp'
console.log(process.env.PGDATABASE); // 'default_db'

// this fails, as it tries to connect to 'yotherdb' after striping the first character.
pg.connect('my_other_db', function (err, client, done) {
});

//a hacky workaround.  Works because it strips the '.' and leaves the rest of the db.
pg.connect('.my_other_db', function (err, client, done) {
}); 

@phated
Copy link

phated commented Sep 12, 2014

This looks good to me. What do you think @brianc?

@brianc
Copy link

brianc commented Sep 13, 2014

I think this looks great. 😄 Definitely something to merge it. It's got tests and it's backwards compatible to boot!

phated added a commit that referenced this pull request Sep 13, 2014
Support usage of relative urls to set database on the default host
@phated phated merged commit c502985 into iceddev:master Sep 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants