-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI] Allow injecting ENV parameters at runtime using %env(MY_ENV_VAR)% #19681
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,11 +11,12 @@ | |
|
||
namespace Symfony\Component\DependencyInjection; | ||
|
||
use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException; | ||
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; | ||
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException; | ||
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException; | ||
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; | ||
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; | ||
use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag; | ||
use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag; | ||
|
||
/** | ||
|
@@ -70,13 +71,14 @@ class Container implements ResettableContainerInterface | |
protected $loading = array(); | ||
|
||
private $underscoreMap = array('_' => '', '.' => '_', '\\' => '_'); | ||
private $envCache = array(); | ||
|
||
/** | ||
* @param ParameterBagInterface $parameterBag A ParameterBagInterface instance | ||
*/ | ||
public function __construct(ParameterBagInterface $parameterBag = null) | ||
{ | ||
$this->parameterBag = $parameterBag ?: new ParameterBag(); | ||
$this->parameterBag = $parameterBag ?: new EnvPlaceholderParameterBag(); | ||
} | ||
|
||
/** | ||
|
@@ -372,6 +374,33 @@ public static function underscore($id) | |
return strtolower(preg_replace(array('/([A-Z]+)([A-Z][a-z])/', '/([a-z\d])([A-Z])/'), array('\\1_\\2', '\\1_\\2'), str_replace('_', '.', $id))); | ||
} | ||
|
||
/** | ||
* Fetches a variable from the environment. | ||
* | ||
* @param string The name of the environment variable | ||
* | ||
* @return scalar The value to use for the provided environment variable name | ||
* | ||
* @throws EnvNotFoundException When the environment variable is not found and has no default value | ||
*/ | ||
protected function getEnv($name) | ||
{ | ||
if (isset($this->envCache[$name]) || array_key_exists($name, $this->envCache)) { | ||
return $this->envCache[$name]; | ||
} | ||
if (isset($_ENV[$name])) { | ||
return $this->envCache[$name] = $_ENV[$name]; | ||
} | ||
if (false !== $env = getenv($name)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to define "providers" instead of calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Really good idea! I suggest to focus on env vars in this PR and open for more env-like providers next. |
||
return $this->envCache[$name] = $env; | ||
} | ||
if (!$this->hasParameter("env($name)")) { | ||
throw new EnvNotFoundException($name); | ||
} | ||
|
||
return $this->envCache[$name] = $this->getParameter("env($name)"); | ||
} | ||
|
||
private function __clone() | ||
{ | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolas-grekas dumb question but why not having the default parameter value as a second argument here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for what purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks more elegant and natural than doing something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry but I don't understand. If you're suggesting to use
%env(FOO, default)%
, then again, this is a parameter name, not a DSL.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm suggesting to be able to do:
instead of the workaround above, although the gain might be limited by the difficulty of parsing the default value. I guess I'm just being a bit to envious on Laravel way of doing things there:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, fully agree. It's really nice that
parameters.yml(.dist)
can declare defaults. See e.g. symfony/demo@21fcb92There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my current project I dropped parameters.yml completely. I think its just unnecessary when using environment variables. For development I am using dotenv to bring the default values…
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah actually I want to get rid of
app_dev.php
as well so using dotenv, although doable it's a bit annoying (dotenv should not be used in production).But the answers that have been given are quite helpful, thanks for it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but you shouldn't have unset environment variables in production either.
I think .env for development is a pretty good solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. The
.env
file will only be loaded if we are in debug mode and environment is not prod. So in prod, I specify every variable in my container configuration e.g.docker-compose.yml
. For running unit tests or acomposer install
while building the container I do an export on the.env.example
file which is similar to the previouslyparameters.yml.dist