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

Override predefined variables #38

Closed
wants to merge 1 commit into from
Closed

Override predefined variables #38

wants to merge 1 commit into from

Conversation

locochris
Copy link

ie. so changing values in .env actually works

The problems we're trying to solve with dotenv:

  • we want settings from environment variables to come in to our app [PASS]
  • we're using unicorn and restarting (with zero downtime), but we need our
    environment variables to be override-able in each new fork [FAIL - the values
    are inherited from their parernt]

Which is why we need to change the current behaviour of dotenv to override predefined variables.
(Perhaps there is a way of configuring the gem to have either behaviour?)

ie. so changing variables in .env actually works
@bkeepers
Copy link
Owner

bkeepers commented Jun 5, 2013

I was thinking about how to solve the unicorn problem a few weeks ago and didn't have any great ideas.

I think it's really important to not override defined env variables by default. Any ideas on how to make this configurable?

@markrebec
Copy link
Contributor

One option, while not very elegant, would be to add a Dotenv.parse method (extracted from load) which would allow someone to parse their .env file(s) and unset any keys found from ENV in the appropriate location for their circumstances. Maybe in a config initializer or somewhere similar (although I don't know that would work for unicorn, as I don't think they reload initializers?).

This would keep the functionality totally abstracted from dotenv itself, and allow people to plugin a simple call to something like Dotenv.parse.keys.each to loop through and unset keys from ENV wherever they might need to.

As an example, I've got a gem I use to work on ecosystems of multiple projects, that contains some scripts and commands to "do things with" the projects that are part of the ecosystem. One of those things is a launcher script that can be used (like foreman) to run processes in a Procfile. Those launchers are actually kicked off by foreman at the root of the ecosystem, so I have to do some unsetting of a couple ENV vars there to make sure they're overridden by the app being launched. Having something here to be able to parse out the keys/vals in each project's .env file and manually unset them would make something like this way more flexible.

Bundler.with_clean_env do
  app.chdir do
    if app.dotenv.exists?
      begin
        # We have to unset these because foreman sets them by default, and Dotenv won't override a set value
        %w(PORT RACK_ENV RAILS_ENV).each { |key| ENV[key] = nil }
        require 'dotenv'
        Dotenv.load!
      rescue
        puts "\e[31mError parsing .env file for #{app.title}.\e[0m"
        exit 1
      end 
    else
      puts "\e[33mWarning: No .env file found in #{app.title}!\e[0m"
      puts "Your application may not behave as expected without a .env file."
    end
  end
end

@markrebec
Copy link
Contributor

I just realized the same restrictions for being able to configure that behavior (dotenv is loaded before initializers are run, so where would you configure dotenv behavior) also sort of apply to unsetting stuff manually... If you were to do that in an initializer, it would just be unsetting the values completely since dotenv would've already tried to load. You could of course call Dotenv.load! after you unset things, but then there's a chance there was an initializer or some other code that executed before your dotenv initializer.

This is a tough one. It seems any configuration would need to be done prior to launching an application + dotenv autoloading, is that right? In that case the only options I see are a config file or an env var (both of which seem kinda funny for a library which serves a very specific purpose of loading a config file and populating env vars).

@markrebec
Copy link
Contributor

Sorry, not trying to spam here, but just had one more thought - what about a custom require for the gem?

It might be a little awkward (?), but maybe something like gem 'dotenv-rails', :require => 'override' would be the way to turn on this functionality, and then that can set a flag prior to requiring the rest of the dotenv files. I think that might work.

@locochris
Copy link
Author

@bkeepers, oh so I'm not alone with the unicorn problem. I couldn't find examples of anyone
else having a solution and was worried I was just "doing it wrong".

@markrebec interesting solution with the custom require. Are there any other gems offering up alternate
behaviours that way in the wild? Or is it worth just making another gem dotenv-rails-override and put
another gemspec in that repo?

@markrebec
Copy link
Contributor

I'm not aware of any other gems controlling configuration or behavior in that way, and the more I think about it, the less fond I am of the solution. I started fleshing out the idea in #41 but it seems extremely restrictive and kind of overkill to control one single aspect of configuration this way, and it doesn't provide any flexibility for the future if there are other behaviors or configuration options we want to control.

The idea of another gem I think may be a bit more in the right direction, but it still has the same pitfalls as the custom require unfortunately.

@bkeepers
Copy link
Owner

Yeah, I'm not loving any of the options at the moment. That's not to say I don't think dotenv should handle it, but just that I haven't seen anything that feels right.

Another idea is to make dotenv more stateful, keeping track of which env vars it loaded, and allow calling Dotenv.reload to re-load the vars it set.

@keithpitt
Copy link

👍 for Dotenv.reload

@bkeepers
Copy link
Owner

It's important to not override defined environment variables. The environment should know better than the app. So I'm going to close this for now. Please feel free to start another discussion about ways to handle reloading.

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

Successfully merging this pull request may close these issues.

4 participants