Need a way to restart server on important environment variable changes

[joshuaflanagan]

A lot of people have been bitten by the fact that spring doesn't reload application config (database.yml, config/initializers) with new environment variables. DATABASE_URL is a perfect example. There is a PR that attempts to solve part of the problem (#267), but even the comments there indicate there is still room for confusing behavior.

We need a way to specify a set of environment variables that should trigger a full reload. By default, it may just include DATABASE_URL, but you should be able to add your own environment variables to that list, in case you use them in initializers.

I tried diving into the code to create a PR, but don't have a strong enough understanding of the different environments in play. I wrote a failing test that demonstrates the problem.

test "changing specific environment variables should reload all configuration" do
  File.write(app.path('config/initializers/set_foo.rb'), <<-CONFIG)
     Rails.application.config.foo = ENV['FOO']
  CONFIG

  app.env['FOO'] = 'first'
  assert_success "bin/rails runner 'p Rails.application.config.foo'", stdout: "first"

  app.env['FOO'] = 'second'
  assert_success "bin/rails runner 'p Rails.application.config.foo'", stdout: "second"
end

As an example, I can get this to pass by making the following change:

# lib/spring/client/run.rb
def call
  if true # check if important environment variable changed
    stop_server
    cold_run
  elsif env.server_running?
    warm_run
  else
    cold_run
  end
rescue Errno::ECONNRESET
  exit 1
ensure
  server.close if @server
end

Obviously, that isn't what we want, since it reboots the server every time. But if we could detect the ENV change at that point, we could conditionally restart everything.

[grosser]

are there env vars that change and should not trigger a reload ?

[joshuaflanagan]

I don't know. I originally thought it should reload on any ENV change, but wasn't sure what kind of unintended consequences that might have. The full reload is pretty drastic (kills all spring perf benefits), so I figured we should be conservative with it and only use it for values we know we care about.

But I think not-respecting environment variable changes is a major violation of "principle of least astonishment" and people will collectively continue to lose hours of productivity until it is addressed.

[grosser]

@jonleighton @rafaelfranca thoughts on this env whitelisting vs blacklisting <-> restart ?

[grosser]

I don't know of any usecase that has a constantly changing environment, so I'd prefer restart on every change and wait for someone to complain ... log message should be nice and clear like "ENV XYZ change detected, restarting"

[rafaelfranca]

I think we can try that.

[grosser]

@joshuaflanagan PR time ? :)

[joshuaflanagan]

I'm going to need more guidance to get to a PR. I spent some time trying to solve the problem this morning, got that failing test, and wrote the naive code that makes it pass, but couldn't figure out how to compare the old/new ENV values at that point in the code (Spring::Client::Run#call).

I'd be happy to work on a PR, but need a nudge on the best way to get access to the old and new values at that point. Or if there is a better place to do the comparison and still trigger a reload. For example, it is very easy to detect if the ENV has changed here

spring/lib/spring/application.rb

Line 145 in 770b2b1

args, env = JSON.load(client.read(client.gets.to_i)).values_at("args", "env")

, but I don't know how to trigger a full reload once we've gotten that far. Feels like the reload needs to be triggered earlier.

[matthewd]

On my machine, PATH & TERM_SESSION_ID change between terminal windows.

PWD, OLDPWD, COLUMNS, and LINES change in a predictable fashion.

I have no idea what Apple_PubSub_Socket_Render and SECURITYSESSIONID are, or when they change.

Some more guaranteed changes: SSH_AUTH_SOCK, SSH_CLIENT, SSH_CONNECTION, SSH_TTY, XDG_SESSION_ID.

---

PATH is because I have special magic... but all the rest seem like they'd be universally annoying.

[jonleighton]

@matthewd makes a good point...

Could we perhaps detect which ENV vars are accessed during application initialization? I don't know if that would work, but seems worth investigating...

[joshuaflanagan]

I don't see how we could, without doing some voodoo like monkeypatching ENV#[] and ENV.fetch, but that seems very heavy-handed. I think a configurable whitelist or blacklist with smart defaults would do the job with minimal invasiveness.
People will still get bitten, but they'll google for the issue, find an answer that suggests updating the black/white list, and now they've fixed the problem for their whole team going forward. Which is much better than the current situation, where everyone can continue to run into the problem and has to remember to keep killing spring manually.

[grosser]

are you suggesting something like adding Spring.watch_env 'MY_SETTING' to config/spring.rb ?

[joshuaflanagan]

That is exactly what I had in mind.

[black-snow]

Still biting me a year later.

[thedanotto]

Same. I remove spring from my applications for this reason. Is there any way to force spring to fully reload? If I can do it manually, I will not continue to remove it.

[sekmo]

how to run a watchdog? I'm tired to run spring stop :-)