Vincit / knex-db-manager

Utility for create, drop, truncate etc. administrative database operations.
https://vincit.github.io/knex-db-manager/
ISC License
142 stars 28 forks source link

databaseManagerFactory doesn't work when the Knex config uses a database URL as the connection property #76

Open JReinhold opened 4 years ago

JReinhold commented 4 years ago

The problem

In Knex, it is possible to define the connection as a single database URL, like:

{
  client: 'pg',
  connection: 'postgres://user:password@localhost:5432/database',
};

This is not supported by the databaseManagerFactory, it requires that the connection property is on the object form, like:

{
    client: 'pg',
    connection: {
      host: 'localhost',
      port: 5432,
      user: 'user,
      password: 'password',
      database: 'database',
    }
}

This makes it inconsistent with Knex. If it is too hard to pull in Knex' way of reading the config, an alternative would be to just document this shortcoming.

Workaround

For what it's worth, I get the values by running RegExp on the database URL, and it works:

const [
  ,
  user,
  password,
  host,
  port,
  database
] = /:\/\/(.*?):(.*?)@(.*?):(.*?)\/(.*)/u.exec(knexConnection);

const databaseManager = databaseManagerFactory({
  knex: {
    client: 'pg',
    connection: {
      host,
      port,
      user,
      password,
      database,
    },
  },
  dbManager: {
    ...
  },
});
elhigu commented 4 years ago

Currently knex-db-manager does not support

connection: 'postgres://user:password@localhost:5432/database',

connections, but connection object is always needed. There is plans to support this, but I'm not sure yet if I'm going to implement it to knex side or here.

There is feature (universal connection string format that is parsed to connection object) being planned in knex side, which would help implementing also this.

JReinhold commented 4 years ago

There is feature (universal connection string format that is parsed to connection object) being planned in knex side, which would help implementing also this.

It makes sense, it should probably be parked then, until it has been built.

nabilfreeman commented 4 years ago

On OS X I have it configured with the connection string and it's working - Knex version 0.17

elhigu commented 4 years ago

@nabilfreeman there are many things that shouldn't work like that, because internally knex-db-manager is trying to read database name etc. from the config object.

nabilfreeman commented 4 years ago

Got it, probably because I was only using createDb and dropDb. I guess adding migrateDb would break.

p.s. I don't think these are off-topic. It's very related to the above discussion.

elhigu commented 4 years ago

It is off topic because it did state that it would actually work (which I had already mentioned above that it is not supported), which is not true. I'd rather hide that by default and keep issue talking about the feature that it should implement and how to implement it.