NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Dotenv] Add support for overriding existing envs#23720

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

voronkovich wants to merge2 commits intosymfony:3.4fromvoronkovich:dotenv-override-existing-envs

Closed

[Dotenv] Add support for overriding existing envs#23720

voronkovich wants to merge2 commits intosymfony:3.4fromvoronkovich:dotenv-override-existing-envs

Conversation

Copy link

Contributor

voronkovich commentedJul 30, 2017•
edited
Loading

Q	A
Branch?	3.4
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#23723
License	MIT

Every time I changed my.env file I need to restart the WebServerBundle's server, because the Dotenv component doesn't override the existing environment variables. This PR adds a dedicated method to overload the environment variables.
Having an ability to overload the.env file, we could solve the issue like this:

// public/index.phpif (getenv('APP_DEBUG') &&file_exists(__DIR__.'/../.env')) {    (newDotenv())->overload(__DIR__.'/../.env');}elseif (!getenv('APP_ENV')) {    (newDotenv())->load(__DIR__.'/../.env');}

Add support for overriding existing envs

eb0749b

carsonbot added Status: Needs Review Feature labels

Jul 30, 2017

voronkovich closed this

Jul 30, 2017

voronkovich reopened this

Jul 30, 2017

Copy link

Contributor

Taluu commentedJul 31, 2017

It if it is in a.env, the DotEnv overwrites only if the var doesn't exist. When you do aputenv (and manipulate$_ENV,$_SERVER variables), it affects only the running process, and does not modify it on the global for all the runs.

So basically, this means that the problem is elsewhere. I'm 👎 for this patch in particular.

Copy link

Member

chalasr commentedJul 31, 2017

Other libraries address this with a dedicated method, that is better IMHO. See
https://github.com/vlucas/phpdotenv/blob/master/src/Dotenv.php
https://github.com/bkeepers/dotenv/blob/master/lib/dotenv.rb#L30

Add a dedicated method to override existing envs

bf32646

voronkovich changed the base branch frommaster to3.4

July 31, 2017 12:17

Copy link

ContributorAuthor

voronkovich commentedJul 31, 2017

@chalasr, thanks for the links! I've added the dedicated method as you suggested!

voronkovich closed this

Jul 31, 2017

voronkovich reopened this

Jul 31, 2017

chalasr added this to the3.4 milestone

Jul 31, 2017

chalasr added the Dotenv label

Jul 31, 2017

Copy link

Member

fabpot commentedAug 1, 2017

While I agree that the references bug should be fixed, I'm 👎 for adding such a method which I consciously decided to not add when I design the library.

Copy link

Member

chalasr commentedAug 2, 2017

@fabpot The first approach was a flag set through the constructor. Personally I think we don't need such state hence suggesting a new method (a new arg for theload() method would do the job as well).
Refreshing env vars for each request cannot be done using DotEnv if we don't want it to be able to overload (or clean up) already defined env vars.