Allow migrations to be used without environments

[leore]

Can some add an example of how to add the migrations object. We just switched from .5 to .6 and are having a hard time figuring out what it is not working.

[bendrucker]

Do you mean a migration itself or the new way of configuring the migrations API with client initialization?

[leore]

Client initialization.

On Tuesday, June 17, 2014, Ben Drucker notifications@github.com wrote:

Do you mean a migration itself or the new way of configuring the
migrations API with client initialization
http://knexjs.org/#Installation-migrations?

—
Reply to this email directly or view it on GitHub
#328 (comment).

[bendrucker]

Migration config takes directory and tableName as previously. The docs list database as well — pretty sure this is a mistake (@tgriesser?). The only changes are:

1. The migration API can borrow from the configuration of the client — Knex.initialize(config). Migration-specific options are passed as config.migrations.
2. The CLI is more useful because it uses process.env.NODE_ENV to pick a key from knexfile.js to look for config. That makes it a lot easier to use the CLI in conjunction with the API.

[leore]

so there is now a new key "migrations" object. directory and tableName used to be keys of the config. If you move those 2 keys under migrations the code breaks. I think something changed with the way the directory is looked up. Either it was absolute and is now relative or vice versa but it does not work but copying the directory and tableName and moving it under a migrations object.

[tgriesser]

Can you paste a rough example of your config just so I can make sure we're on the same page

[pon]

I'm working with @leore. We resolved the issue. Here is the working config:

database: {
    client: config.get('DATABASE:CLIENT'),
    connection: {
      host: config.get('DATABASE:HOST'),
      user: config.get('DATABASE:USER'),
      password: config.get('DATABASE:PASSWORD'),
      database: config.get('DATABASE:DATABASE'),
      port: config.get('DATABASE:PORT'),
      charset: config.get('DATABASE:CHARSET'),
      dbcollat: config.get('DATABASE:DBCOLLAT')
    },
    migrations: {
      directory: __dirname + '/migrations',
      tableName: 'migrations'
    }
  }

We were getting errors using a relative path in the migrations directory. Changing to an absolute directory resolved the issue.

[tgriesser]

Okay, cool, glad it's working... I'll need to update that in the docs.

[ravinggenius]

@bendrucker

The CLI is more useful because it uses process.env.NODE_ENV to pick a key from knexfile.js to look for config.

Is it possible to disable this behavior? I'd prefer to manage my own configuration and environments and just use knexfile.js to export an appropriate configuration for the current environment.

So instead of this...

var config = require('./source/config');

module.exports = {
    development: {
        client: config.databaseDialect,
        connection: {
            host: config.databaseHost,
            user: config.databaseUsername,
            password: config.databasePassword,
            database: config.databaseName
        },
        pool: {
            min: 2,
            max: 10
        },
        migrations: {
            tableName: 'schema_info'
        }
    },

    test: {
        client: config.databaseDialect,
        connection: {
            host: config.databaseHost,
            user: config.databaseUsername,
            password: config.databasePassword,
            database: config.databaseName
        },
        pool: {
            min: 2,
            max: 10
        },
        migrations: {
            tableName: 'schema_info'
        }
    },

    production: {
        client: config.databaseDialect,
        connection: {
            host: config.databaseHost,
            user: config.databaseUsername,
            password: config.databasePassword,
            database: config.databaseName
        },
        pool: {
            min: 2,
            max: 10
        },
        migrations: {
            tableName: 'schema_info'
        }
    }
};

... I'd prefer something like this:

var config = require('./source/config');

module.exports = {
    client: config.databaseDialect,
    connection: {
        host: config.databaseHost,
        user: config.databaseUsername,
        password: config.databasePassword,
        database: config.databaseName
    },
    pool: {
        min: 2,
        max: 10
    },
    migrations: {
        tableName: 'schema_info'
    }
};

The only "advantage" to specifying configuration options for each environment that I can think of is migrating all databases in sync, though I don't want that behavior either. An argument in favor of the simpler syntax is that is what is used to establish the connection for my models. Why make it one way here and another way somewhere else?

[leore]

+1 for this. We manipulated the current schema to allow us to pass it in via env variables

[bendrucker]

@ravinggenius @leore I'm with you guys. I don't use environments in any code. I much prefer loading in config from process.env, process.argv, json, in-memory objects, etc. either manually or using nconf where appropriate. Not sure why I described the CLI as more useful because it supports environments above. I think the idea of env for migrations was first raised in #231 where I tried to shoot it down multiple times.

In the mean time, you could definitely be a little DRYer with something like:

exports.development = exports.staging = exports.production = {
  // config props
};

Unfortunately there's not really a way to revert this without breaking a lot of implementations. To be honest I don't use the CLI at all. Since I'm using Gulp as a task runner anyway for big projects I find it much easier to just call the migration API directly from a task.

[bendrucker]

Closing this since I don't see a better way of handling things now that environments are baked in for those who don't want them. Feel free to continue discussion though for ideas, examples, etc.

[tgriesser]

I'm sure we could come up with something where we could fallback to treating module.exports as a config object (if I'm reading correctly that that's what people are after)

[bendrucker]

It's one of those things that can cause weird bugs. Might still be the right move. Imagine a config object like this:

{
  "production": true
}

Not the most likely bug but if it bit you it would be insanely annoying and a huge pain to track down. Logging out the detected environment and checking that the environment export is an object should make it pretty unlikely though.

[gaastonsr]

@bendrucker I like your last idea. Would you consider a PR?

[bendrucker]

Yes!

[tkellen]

Did this actually work #328 (comment)?

I'm not seeing anything in master that looks for the migration configuration under a migrations key.
https://github.com/tgriesser/knex/blob/master/lib/migrate/index.js#L119
https://github.com/tgriesser/knex/blob/master/lib/migrate/index.js#L213
...etc

[gaastonsr]

There is a PR but it's not merged yet.

On Tue, Oct 21, 2014 at 7:22 PM, Tyler Kellen notifications@github.com
wrote:

Did this actually work #328 (comment)?
I'm not seeing anything in master that looks for the migration configuration under a migrations key.
https://github.com/tgriesser/knex/blob/master/lib/migrate/index.js#L119
https://github.com/tgriesser/knex/blob/master/lib/migrate/index.js#L213

...etc

Reply to this email directly or view it on GitHub:
#328 (comment)

[kimmobrunfeldt]

I also define all of my environments with a same config which takes some specifics from env variables. Now I have to do this:

module.exports = {
    local: config,
    ci: config,
    dev: config,
    qa: config,
    prod: config
};

If a new environment is added, worst case is that migrations fail in the new environment because it's not defined in knexfile.js. It would be best if all the envs should not be defined excplicitly.