feat(config): Local environment configurations

[mleanos]

EDIT: 10/29/2015

This PR has changed quite a bit, and originally was not properly described. Please see the below commit, and discussion.

Enables the setting of the test database configuration inside of the local environment config. If this setting is present, then it will override the db setting in the Test environment config. This will compliment the already existing db setting in local.js.

I've also added environment variables to Production, Test, and Development:
process.env.MONGO_USER
process.env.MONGO_PASS

This is both an enhancement, and support feature. Newer versions of MongoDB are deprecating MONGODB-CR authentication. Going forward it is best practice to provide the user & pass options with the mongoose connect method.

This also enhances the experience of a meanjs user, by providing an easy way to manage their connections between dev & test databases. For someone (like me) using MongoLab, the ability to pass the connection options to mongoose will save a lot of time during development/testing.

For me, the way I have been going about managing my testing database is by manually setting MONGOLAB_URI to my URI; which includes the user & pass. However, once Mongoose is upgraded to a version that no longer supports this, I would lose the ability to connect my mean app to my Test database.

I set this PR as a WIP because I anticipate much debate about this implementation. However, I would really like to get this conversation going. We can't avoid Mongoose upgrades for too long.

[mleanos]

An example of when a Mongoose upgrade causes database connectivity to break.. #1010

[ilanbiala]

why testDb?

[mleanos]

@ilanbiala Are you questioning the name? Or the purpose?

[ilanbiala]

Name

[mleanos]

what would be a better name?

[mleanos]

We need to differentiate between the dev database, and test database.

Or were specifically asking about the name? I couldn't think of anything better.

[mleanos]

I know we're having issues with Mongoose version support. However, this PR doesn't curtail those issues. This can help workaround the issues for a user running the app locally though. This PR is more for a convenience/support feature during local development.

[ilanbiala]

@mleanos this just adds another database to the local env. What does that really offer?

[mleanos]

@ilanbiala It doesn't just add any other database to the local env config. It offers the ability to specify a database that will be used by the Test env; other than a local database. The reasoning for this addition, is the same as why the db setting was added to the local env.

There may even be cases where a user would want to test multiple databases, that reside in remote locations. This would offer that. The alternative would be to have the user manually modify the Test env config settings, or env variables.

The reason why the database used in the Test env needs special handling, is because the local.js config isn't merged with the current env config.
https://github.com/meanjs/mean/blob/master/config/config.js#L193

[ilanbiala]

@mleanos doesn't it make more sense to fix that rather than add another field to circumvent that?

[mleanos]

@ilanbiala I realize I had a typo in my last comment. It should have read..

"... is because the local.js config isn't merged with the Test env config."

This is why the test database would need special handling. Maybe this is a better explanation of what I'm trying to accomplish here...

Currently, the Development, and Test, environment configs have a field called db. When the application is ran, in either of these environments, the config would use the db field for connecting to the database. The database is set from a environment variable (either process.env.MONGOHQ_URL, or process.env.MONGOLAB_URI), otherwise it builds a URI for a local mongodb instance. As seen here.. https://github.com/meanjs/mean/blob/master/config/env/development.js#L7

For someone that doesn't have a local instance of mongodb, or wants to connect to a remote db host (like MongoLab), they would need to set a environment variable. Having to manage the local environment variables is a cumbersome task & slows development. Thus, why the local.js was setup to allow the db field to override the Development/Production env setting. This works great, and makes development much easier & provides a way to connect to remote databases; this could even be used in a Production environment if the user chooses to.

So what I'm proposing is to provide the same exact functionality for the Test environment database setting. However, we can't simply use the db setting in the local.js file; unless the user manually toggled these settings when they were switching back & forth between Test & Development mode. This is why I added the testDb field; we have to differentiate this from the db to be used with Development/Production. Furthermore, we can't expect the user to modify the Test env config file, just as we can't expect them to modify the Development file.

[ilanbiala]

@mleanos I'm still getting lost in what this is trying to accomplish, maybe try simplifying with an example?

[mleanos]

@ilanbiala Let's say you're developing on your local machine..

If you are actively testing/developing, using grunt, and grunt test. Currently, config/env/local.js has the setting db that will override the setting in config/env/development.js. This is a very handy convenience feature that allows a user to define the database that will be connected to, when grunt is ran. There's no such capabilities for when a user runs grunt test.

A user must either have an environment variable set, or a local instance of mongodb. Even if the user does have a local instance of mongodb, they may want to periodically/actively switch to a remote database, or even a different local instance (different database by name).

By adding a setting in config/env/local.js that will be used for to connect to the mongodb when grunt test is ran, the user can easily define exactly what database to use. I don't want to set a environment variable each time I want to change the database that will be used with the Test config; nor do I want to update config/env/test.js. This is the purpose of the local.js file; just as it's being used currently for the Development database setting. This file never gets committed; it's something I use quite frequently during development for overriding certain settings, and it's very useful as a contributor (I don't need to update any core config files).

The example setting is in this PR, in config/env/local.example.js

[ilanbiala]

Aside: This part of the comment doesn't really pertain to this.

What's wrong with having the default db be localhost and then you never have to worry about not having a db?

[mleanos]

@ilanbiala What if the user doesn't have mongodb installed locally? localhost won't exist.

[ilanbiala]

@mleanos I don't see why that wouldn't be something that we could ask instead of all this. I get what this is doing now, but I'm really not fond of how we go about it, and I think it's too much of a stretch when you can just avoid the issue.

[mleanos]

@ilanbiala It's not about avoiding an issue, or not. There are many use cases that this may useful for.

Think of the reason why we have the db setting in local.js for use when the app ran with Development. Now apply that same reasoning to someone running the app with Test env.

[lirantal]

@mleanos I think this PR wasn't described properly so it took me some time to figure out what you're after.

You basically saying that you'd like to specify custom test environments for both test and development? so that when you are running your grunt test you won't be using the test.js env config but rather a specific one to your environment.

Seems to me like a valid point and useful in my cases as well. I would even extend it to also override the production development environment if someone wishes to do so in local.js and not commit that file to git.

[mleanos]

@lirantal I apologize for the confusing commit message, and the branch name :| It was created when I thought the sky was falling with Mongoose ;) I realize it may have been why this PR is confusing.

And yes, I think you get my point. I want the same functionality that db in local.js is offering when the NODE_ENV is Development, for when the NODE_ENV is Test; for now I am after just specifying the database to be used.

@jloveland had a good idea on restructuring local.js.

Something like

{
  prod: {},
  dev: {},
  test: {}
}

We would need a function in config.js that properly overrides any settings in local.js for the current NODE_ENV. Is that what you were driving at?

[lirantal]

@mleanos sounds like a good idea, or do you think it will be more organized to have:
local.prod.js, local.dev.js, local.test.js ?

[mleanos]

I think I like your idea of breaking them into their own files. It seems like it would be easier for a user to manage that way.

Thanks a bunch! Glad you thought of it, cuz it didn't even cross my mind :)

[lirantal]

Cool.
Do you want to update this PR or let's wait until some more people can comment before you spend any time on this change? @jloveland @ilanbiala @ryanjbaxter

[jloveland]

+1 for local.js files to specify different configs for each environment.

[ryanjbaxter]

+1 sounds like a good idea, many people when using CF have a dev, test, and production versions deployed as well.

[ilanbiala]

I'm not really too fond of having many files for one environment based on NODE_ENV, but I can't think of a much better way.

[mleanos]

@lirantal @ilanbiala I have updated this PR per our discussion. I've also squashed down to one commit to avoid any confusion, since the previous commits are no longer applicable.

I'm wondering if we could get this into 0.4.2. The only possibility of a breaking change is that a user's previous local.js file will not be picked up by this change. That seems very minor, IMO. WDYT?

[codydaig]

@jloveland What about the existing local.example.js file that we do want to commit into the repo?

[jloveland]

@codydaig, apologies, you are correct because the grunt copy function needs the local.example.js

[mleanos]

@jloveland Thank you for the review, and for being the catalyst that got this implementation conceptualized :)

I originally thought that the additional local-ENV files should be created as well. Since the copy:localConfig task is ran against the other grunt tasks specific for the other environments. However, I started to question why the task existed in the first place. Nothing is relying on it, and nothing is using it once it gets created. In fact, the whole file is commented out before the user goes in and sets it up themselves.

Even though there's no tangible need for the copy:localConfig task, I'm actually fine with it merely generating the local-development.js file for ease of demonstration. So if we're going to keep the copy task, I say we just limit it to the development file.

We definitely will need to add some documentation for this. I tried putting as much into local.example.js as I could, but the gh-pages will need it too.

[jloveland]

SGTM!

[lirantal]

LGTM.

[codydaig]

LGTM

[ilanbiala]

Awesome, we'll merge once we release 0.4.2 and start working on 0.5.0

[mleanos]

SGTM. Thanks @ilanbiala!

[mleanos]

Looks like the build is failing on [email protected]
https://travis-ci.org/meanjs/mean/jobs/90825637#L733

Once 0.4.2 is released, I'll look at this further.

[ilanbiala]

Sounds good.

[mleanos]

@lirantal This is ready to be merged, now that we have 0.4.2 released.

[lirantal]

Indeed, I'll close and re-open so we can re-run CI to make sure everything is ok.

[codydaig]

@mleanos Can you rebase too verify that everything is up too date and working?

[lirantal]

@mleanos build fails on node v4 test.

[mleanos]

Ok. Thank you. I'll rebase, and test again.

[mleanos]

@lirantal @codydaig Build is passing now. I forgot to look at the build error, before I pushed the rebase. It may have been an issue with the CI config changes that I didn't have in this branch (or possibly that Docker fix that got merged).

Either way, it seems to be fine now :)

[codydaig]

LGTM

[mleanos]

Thanks guys!

To anyone that uses this, let me know how it's working out with your development flow. I'm curious to know from other developers perspective. Now I can get rid of all my crazy logic/functions in my local.js file :)