spring
spring copied to clipboard
Assign envs before preload
Currently ENV's of the client are not up to date during initialization of the app.
When having the following in config/application.rb
...
config.force_ssl = !!ENV['FORCE_SSL']
...
The following occurs:
FORCE_SSL=1 RAILS_ENV=test bin/rails runner 'p Rails.application.config.force_ssl' # => trueRAILS_ENV=development bin/rails runner 'p Rails.application.config.force_ssl' # => true
If you reverse these steps, the result for both steps would be false.
Obviously we want 1 to yield true, and 2 to yield false.
Thanks for the PR, some comments:
- Have you thought about what happens if the application process gets reloaded? I.e. if you
touch config/application.rb, when then will be theforce_sslvalue of the development environment? (lib/spring/application/boot.rbwill calleager_preload) - Now that we're mutating the environment in the parent process, I think it would be worth doing the mutation where we need it, and then reverting
ENVback to its previous value - my concern is that just leaving it as it is may give rise to other bugs. - Please write to the initialiser in the actual test rather than in
helper.rb, as it only relates to this one test and doing it in the test makes it clearer what is going on. - Please add to
CHANGELOG.md(under a new "Next release" heading)
Thanks!
Thanks @jonleighton for the extensive feedback!
I took a stab at implementing this - but I'm struggling to make the build green :poop:
It's a bit unclear to me how the different env's (original_env, ENV and client_env) should be treated. Could you take a look and post some more pointers/comments?
I took a look at this the other day and realised that it needs some more fundamental changes to make it work properly. I'll come back to it at some point when I can, but it might take me a while I'm afraid.
If I can help in the process; let me know! :cake:
@jonleighton can we give this PR another try? I rebased and updated the code to make it pass.
The original issue is solved now: application:<rails_env> gets initialized with the ENV from the command that starts it.
This is not perfect as we still don't fully 'watch' ENV (for subsequent commands using the same rails-env), but it's much better than the current situation where you simply cannot rely on ENV anywhere in config/**/*.rb.
WDYT?
@eval Sorry for the slow reply. I'll take another look.
@eval This looks good to me. Could rebase it please? Could you also document how we treat the ENV in the README? There are some nuances so I think it would prevent confusion to spell this out to people (the main one being that if you start the app with FORCE_SSL=1 and then set FORCE_SSL=0, it won't do what you expect). Thanks for your patience.