Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Environment variables in config file #270

Open
aaronmoodie opened this issue May 13, 2015 · 13 comments
Open

Environment variables in config file #270

aaronmoodie opened this issue May 13, 2015 · 13 comments

Comments

@aaronmoodie
Copy link

aaronmoodie commented May 13, 2015

I have separate config json files for dev and prod, using the format

{
    "user": USER,
    "password": PASSWORD,
    "database": DATABASE_NAME
}

Using the command db-migrate up --config config/production.json -e prod how would I reference USER and PASSWORD from production.json in my database.json config file?


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@wzrdtales
Copy link
Member

use one configuration file like in this example and differentiate your environments directly:
http://db-migrate.readthedocs.org/en/v0.9.x/Getting%20Started/configuration/#configuration

@aaronmoodie
Copy link
Author

thanks @wzrdtales
I've broken the files up so that I can include the right one using NODE_ENV. In the same dir as config/production.json and config/development.json I have an index.js which is requiring the appropriate json file for the environment.

module.exports = require('./' + (process.env.NODE_ENV || 'development') + '.json');

Then elsewhere in my app I can load in the right user and password:

var config = require(root + '/config');
var password = config.password

Is there a way I can work this setup in with my db-migrate database.json file?

@TGOlson
Copy link

TGOlson commented May 19, 2015

+1 - I have the same issue. My configs are structured almost identically to what @aaronmoodie described.

Some more dynamic configuration process would be nice, or at the very least making the config file optional so that I can use my own internal tools for exposing a database.

@wzrdtales
Copy link
Member

@TGOlson @aaronmoodie The latter one doesn't sound like an option, the only way to "expose" the database by your own tools, would be to pass it in some way to db-migrate. This means you would need to integrate db-migrate with your tools.

So, ok. Let's have a look together at the current https://github.com/db-migrate/node-db-migrate/blob/master/lib/config.js.

As you might notice, the config gets required. This means the config file has not necessarily be a .json file. What you are, in fact already be able to do, is to write a little middleware that interprets your current configuration setup.

To give an example, imagine the following config.js file:

var config = {
  production: require('production.json'),
  development: require('development.json')
};

module.exports = config;

This is just an example, I haven't tested it so far, but this should work.

Ok now, let us think about problems that might occur when using it in a way like this. The first thing I can think of, would be that we haven't the information of the current environment in our little config snippet. That could be improved, by checking if the exported module is just an object, or if it is executable, if so pass that variables to the constructor and call the function, lets say named getConfig()

You see, basically you already have the possibility to make the config fit more into your usual workspace. Right now there might be some parts missing, but until this issue there wasn't a requirement for this yet.

Asking for suggestions and open for ideas.

@TGOlson
Copy link

TGOlson commented May 20, 2015

@wzrdtales thanks for the reply, I think your example gives me something to work with.

Also, I don't think I explained my initial idea very well. Injecting a DB connection or ORM object would be really nice (although to your point, a little too complex), but it would also be nice just to skip the config altogether. Something like:

db-migrate --no-config

And then when you write your migration, the db is just an empty object:

exports.up = function(db, callback) {
  // db === {}  
}

@wzrdtales
Copy link
Member

@TGOlson this would not work, db is not the driver of the database, but the driver of db-migrate for this specific database ;) you would have to link the database driver into this driver.

@gregl83
Copy link

gregl83 commented May 26, 2015

I'm using config for my configuration management.

Config has separate environment configuration files so that sensitive data may be omitted from version control systems (staging / production creds etc).

I believe storing creds in env vars is less secure than files with strict access control...

As db-migrate is currently built it would require storing both dev and prod passwords in the same file and require managing that database.json file outside of version control. Also, the dev config isn't needed in the prod env. That would also result in duplicate configurations in many use cases. The only other option is the env vars...

Perhaps implementing config as the configuration management tool as major release???

@wzrdtales
Copy link
Member

@gregl83 There is no "storing" in environment variables.Also I think the objects wasn't intended to be used like you think they're used.

In fact you can utilise this different objects in the json file to define multiple configs for different databases and different dbms's. I do not think that this was planned just for using it to separate test, dev and production though.

Currently to fullfill your requirements, there is the solution I pointed out above already, or include another config file via --config.

About implementing it as a major release, well to release a version with breaking changes, just for another configuration solution that wont fit all use cases, I would prefer if I don't have to do this. Instead we should gather the requirements you all have and try to find the best solution for everyone and in the best case one without breaking anything that exists yet.

@gregl83
Copy link

gregl83 commented May 26, 2015

@wzrdtales, totally get it and thanks for putting the package open source in the first place. I like the idea of keeping the migration process in node vs other solutions available.

Just adding my use case to the mix which seems like may work for both @aaronmoodie and @TGOlson but understand that introducing breaking changes most often is not the best solution.

@TGOlson
Copy link

TGOlson commented May 26, 2015

Hey fellas,

Just swinging by to say we solved our problem by wrapping calls to db-migrate in our build tool, Gulp. We basically load our own internal config, and then provide the environment specific db url to db-migrate. Something like this:

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

var DB_MIGRATE_COMMAND = 'DATABASE_URL=' + config.db + ' ./node_modules/.bin/db-migrate';

gulp.task('migrate', shell.task(DB_MIGRATE_COMMAND));

I omitted some setup code for brevity (if anyone wants the full solution I'd be happy to provide it). We do a little more work to wrap the tasks so you can provide options, like up --count 2, etc., but in general this has worked very well, without having to do much code/data duplication.

Edit: in case it's not clear, the DATABASE_URL environment variable is a variable that can be provided to db-migrate. If you provide this variable, you can omit using a database.json config file. (It's not documented very well in the docs).

@wzrdtales
Copy link
Member

@TGOlson Great to see, you have found a way that works for you and your team.

From what you've done I think, it would help to add a method to the programable API (there is no such API yet, but v0.10.x introduce one) that specifies a config object directly instead of parsing a file?
In v0.10.x things like you have done now, are much easier, as there finally an API gets introduced that is made for the purpose of using db-migrate within another node.js module or in some automation.

Details about this are about to be documented soon, since yet there is only the instruction about the call convention over a singleton.
http://db-migrate.readthedocs.org/en/latest/API/programable/

@TGOlson
Copy link

TGOlson commented May 26, 2015

@wzrdtales that sounds great. It would definitely be ideal to programmatically call an API instead of wrapping shell commands. Looking forward to seeing it.

@antonioreyna
Copy link

calling the db-migrate via command line, how can i set the NODE_ENV so i can load the .env.dev or .env.prod

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants