Add better configuration handling

[jcamenisch]

Goals:

• Any missing, required environment variables should throw an error at application boot time.
• defaults should be configurable in a central place.

[jcamenisch]

I think this approach might work well: https://gist.github.com/jcamenisch/b878c667a894f4d903a2

[ericboehs]

I don't know, this may be overkill. I think if we just always call ENV.fetch('MY_VAR'), we'll know where it was called and that it's missing. If we add a comment to the line in .env.example.

I would like it to throw at boot time though.

[jcamenisch]

Do you have any simpler/lighter suggestions?

One idea: just assign ENV variables to Rails.configuration in application.rb. Downside: This would de-emphasize the difference between environment-specific stuff and other configurations.

Another idea: just assign each env variable to a Ruby constant at boot time, like

AIRBRAKE_API_KEY = ENV['AIRBRAKE_API_KEY'] || '' # 'see http://airbrake.io/'
WIDGET_PRICE = ENV.fetch('WIDGET_PRICE').to_d   # The price for one widget

This would accomplish almost everything we'd want, but wouldn't be quite as helpful in forcing everyone into the right practices.

You could also do it this way, but that relies too heavily on discipline for my taste. It also doesn't provide the self-documentation benefits for each variable.

[ericboehs]

I almost like the last way. Either that or parse over the .env.example and raise if any are missing.

On Sat, Nov 23, 2013 at 8:17 AM, Jonathan Camenisch
notifications@github.com wrote:

Do you have any simpler/lighter suggestions?
One idea: just assign ENV variables to Rails.configuration in application.rb. Downside: This would de-emphasize the difference between environment-specific stuff and other configurations.
Another idea: just assign each env variable to a Ruby constant at boot time, like

AIRBRAKE_API_KEY = ENV['AIRBRAKE_API_KEY'] || '' # 'see http://airbrake.io/'
WIDGET_PRICE = ENV.fetch('WIDGET_PRICE').to_d   # The price for one widget

This would accomplish almost everything we'd want, but wouldn't be quite as helpful in forcing everyone into the right practices.

You could also do it this way, but that relies too heavily on discipline for my taste. It also doesn't provide the self-documentation benefits for each variable.

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

[vlucas]

I had this same issue for the PHP version of dotenv I made. I added a required method that I used in the LifeChurch staff app so it can be more self-documenting if LC staff tries to clone and run the project.

The required method throws an exception if any of the ENV vars are missing:

Dotenv::load(__DIR__);
Dotenv::required(array('DB_HOST', 'DB_NAME', 'DB_USER', 'DB_PASS'));

Maybe we could do something similar, or contribute this to the dotenv Ruby gem too?

[ericboehs]

@vlucas that'd be nice

On Sat, Nov 23, 2013 at 2:10 PM, Vance Lucas notifications@github.comwrote:

I had this same issue for the PHP version of dotenvhttps://github.com/vlucas/phpdotenvI made. I added a
required method that I used in the LifeChurch staff app so it can be more
self-documenting if LC staff tries to close and run the project.

The required method throws an exception if any of the ENV vars are
missing:

Dotenv::load(DIR);Dotenv::required(array('DB_HOST', 'DB_NAME', 'DB_USER', 'DB_PASS'));

Maybe we could do something similar, or contribute this to the dotenv Ruby
gem too?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-29140522
.

[jcamenisch]

Dotenv is for development. I'm pretty sure they wouldn't accept a PR that's
targeted toward a production use case.
On Nov 23, 2013 3:18 PM, "Eric Boehs" notifications@github.com wrote:

@vlucas that'd be nice

On Sat, Nov 23, 2013 at 2:10 PM, Vance Lucas notifications@github.comwrote:

I had this same issue for the PHP version of dotenv<
https://github.com/vlucas/phpdotenv>I made. I added a
required method that I used in the LifeChurch staff app so it can be
more
self-documenting if LC staff tries to close and run the project.

The required method throws an exception if any of the ENV vars are
missing:

Dotenv::load(DIR);Dotenv::required(array('DB_HOST', 'DB_NAME',
'DB_USER', 'DB_PASS'));

Maybe we could do something similar, or contribute this to the dotenv
Ruby
gem too?

—
Reply to this email directly or view it on GitHub<
https://github.com/brightbit/rails-template/issues/2#issuecomment-29140522>

.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-29140683
.

[jcamenisch]

This started with the statement that 48 lines of Ruby was overkill, and now
we're talking about parsing over a .env.example file when booting the
production app? Can you expound a little on what we're trying to shoot for?
On Nov 23, 2013 1:30 PM, "Eric Boehs" notifications@github.com wrote:

I almost like the last way. Either that or parse over the .env.example and
raise if any are missing.

On Sat, Nov 23, 2013 at 8:17 AM, Jonathan Camenisch
notifications@github.com wrote:

Do you have any simpler/lighter suggestions?
One idea: just assign ENV variables to Rails.configuration in
application.rb. Downside: This would de-emphasize the difference between
environment-specific stuff and other configurations.
Another idea: just assign each env variable to a Ruby constant at boot
time, like

AIRBRAKE_API_KEY = ENV['AIRBRAKE_API_KEY'] || '' # 'see
http://airbrake.io/'
WIDGET_PRICE = ENV.fetch('WIDGET_PRICE').to_d # The price for one widget

This would accomplish almost everything we'd want, but wouldn't be quite
as helpful in forcing everyone into the right practices.
You could also do it this way,
but that relies too heavily on discipline for my taste. It also doesn't

provide the self-documentation benefits for each variable.

Reply to this email directly or view it on GitHub:

#2 (comment)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-29138298
.

[jcamenisch]

I think I made a mistake here when I included the functionality to specify
a class with each variable spec, and have the library auto-convert the
string into the desired class. I think I like the feature, but it's not
part of the essence, and it distracts from the comparison of solutions.

To let you know how that got in there, I was looking at some other
libraries for idioms on how to design the configuration API. I ended up
imitating optparse, because it seemed like the pattern would work pretty
well.

Optparse takes a class parameter, and converts the supplied parameter to it.
I liked the idea and threw in an implementation of it, adding several lines
of code.

I'd rip that out for you to see, but that sounds like a pain to do on my
phone. :p

So, this was me playing with some ideas. The API doesn't seem to be hitting
you right, or maybe my vision isn't clear. I'd like to understand more what
you're thinking.

[vlucas]

Good point. All the required method does is check if the ENV var is set, so it's safe for production use. In PHP-land I'd do something like this:

if(BULLET_ENV !== 'production') {
    Dotenv::load(__DIR__);
}
Dotenv::required(array('DB_HOST', 'DB_NAME', 'DB_USER', 'DB_PASS'));

So dotenv is still loaded as a dependency, but it's not loading and parsing a .env file on each request.

[jcamenisch]

This is what the Cfg module looks like without the class conversion functionality: https://gist.github.com/jcamenisch/b878c667a894f4d903a2 —29 total lines instead of 48.

[ericboehs]

Still way confusing.

I was thinking something simple like: ENV.keys.include? Dotenv.load('.env.example').keys

Edit:

Looks like that doesn't work. The proper way to get the .env.example keys is: Dotenv::Environment.new '.env.example'.

Also the way I was doing the array subset check won't work.

It could be done via: (Dotenv::Environment.new('.env.example').keys - ENV.keys).empty?

Working it out a little more:

missing_vars = Dotenv::Environment.new('.env.example').keys - ENV.keys
raise "Missing ENV vars: #{missing_vars}" if missing_vars.any?

This SO question has a couple other ways to do it.

[jcamenisch]

We don't want everything in .env.example to be required. Some things can
have default values or just be left undefined.

What part is confusing? The usage or the implementation? You don't really
need to understand the implementation, as long as the usage is clear.
On Nov 25, 2013 11:54 AM, "Eric Boehs" notifications@github.com wrote:

Still way confusing.

I was thinking something simple like: ENV.keys.include?
Dotenv.load('.env.example').keys

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/2#issuecomment-29218743
.

[ericboehs]

I don't really like the use of a Cfg object when we already have a ENV object.

And the implementation is confusing. I don't see why we should just ignore the implementation and not review it.

[ericboehs]

I don't particularly have a recommendation for adding optional variables aside creating a .env.required file though. I usually prefer the least amount of code. Also I don't see the need to put the description as code. Code comments are fine.