8000 [Runtime] Add dotenv_overload option to SymfonyRuntime to tell Dotenv to override existing vars by fancyweb · Pull Request #41681 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Runtime] Add dotenv_overload option to SymfonyRuntime to tell Dotenv to override existing vars #41681

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[Runtime] Add dotenv_overload option to SymfonyRuntime to tell Dotenv to override existing vars #41681

wants to merge 1 commit into from

Conversation

fancyweb
Copy link
Contributor
Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Proposing this as bug fix because it's retro compatible and it looks like it was "forgotten".

My use case is that I have projects with:

// ...
$dotenv = new Dotenv();
$dotenv->bootEnv($fullDotEnvPath = dirname(__DIR__).'/.env');
$dotenv->overload($fullDotEnvPath);
//...

I think it's not currently possible to migrate this behavior to the new Runtime component.

After:

// ...
$_SERVER['APP_RUNTIME_OPTIONS'] = [
    'dotenv_overload' => true,
];
// ...

nicolas-grekas
nicolas-grekas reviewed Jun 12, 2021
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tell us a bit more about the use case? because real env vars should always win. Don't you have control over them at all? This also means that parsing happens 100% of the time, contrary to what .env.local.php allows. Not sure we want to make this a mainstream option.

@nicolas-grekas nicolas-grekas added this to the 5.3 milestone Jun 12, 2021
@fancyweb
Copy link
Contributor Author
fancyweb commented Jun 14, 2021 •
edited
Loading

Can you tell us a bit more about the use case?

I have to deploy some projects on a "staging" server that contains dozens of projects. Some env vars already exist at the system level (for example: APP_ENV=dev). It's a mistake from the past but I guess some projects out of my perimeter might rely on it. I don't want to break everything and I don't have the time to check/fix every projects. So it's faster and safer to use the "overload" feature for the projects I care about 😄 The performance overhead doesn't matter in my case.

@fancyweb fancyweb force-pushed the runtime/dotenv-overload branch from 08319b2 to 035ac95 Compare June 14, 2021 12:44
@Tobion
Copy link
Contributor
Tobion commented Jul 15, 2021

How about this would be integrated into composer dump-env instead? Having something like composer dump:env prod --immutable which would dump the env but so that it cannot be overwritten by real env vars.

nicolas-grekas
nicolas-grekas reviewed Aug 26, 2021
View reviewed changes
src/Symfony/Component/Runtime/SymfonyRuntime.php
->bootEnv($options['project_dir'].'/'.($options['dotenv_path'] ?? '.env'), 'dev', (array) ($options['test_envs'] ?? ['test']));
->bootEnv($fullDotenvPath, 'dev', (array) ($options['test_envs'] ?? ['test']));
if ($options['dotenv_overload'] ?? false) {
$dotenv->overload($fullDotenvPath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, I would welcome a new argument $overload argument on bootEnv()

Copy link
Contributor Author
@fancyweb fancyweb Sep 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean calling $this->overload() at the end of bootEnv()?

Copy link
Member
@nicolas-grekas nicolas-grekas Sep 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean calling populate() with $overrideExistingVars and doLoad() with the same arg instead of load()

@fancyweb fancyweb mentioned this pull request Oct 26, 2021
Merged
@fancyweb fancyweb closed this Oct 26, 2021
nicolas-grekas added a commit that referenced this pull request Oct 26, 2021
@nicolas-grekas
feature #43755 [Dotenv] Add $overrideExistingVars to bootEnv() and lo…
48b78fe 
…adEnv() and dotenv_overload to SymfonyRuntime (fancyweb)

This PR was merged into the 5.4 branch.

Discussion
----------

[Dotenv] Add $overrideExistingVars to bootEnv() and  loadEnv() and dotenv_overload to SymfonyRuntime

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | #41681 (comment)
| License       | MIT
| Doc PR        | -

The goal is to be able to use `bootEnv()` and `loadEnv()` and override existing vars directly instead of having to call `overload()` in a second time.

Commits
-------

bcb0592 [Dotenv][Runtime] Add $overrideExistingVars to bootEnv() and loadEnv() and dotenv_overload to SymfonyRuntime
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

Assignees
No one assigned
Labels
Bug Runtime Status: Needs Work
Projects
None yet
Milestone
5.3
Development

Successfully merging this pull request may close these issues.
4 participants
@fancyweb @Tobion @nicolas-grekas @carsonbot
0