Nullable environment variable processor #29767
Conversation
src/Symfony/Component/DependencyInjection/Tests/EnvVarProcessorTest.php
Outdated
Show resolved
Hide resolved
|
Appveyor randomly fails on |
|
You can have a look here: #29760 |
|
Reworded commit message and force pushed, everything's fine now 🎉 |
| @@ -195,6 +196,10 @@ public function getEnv($prefix, $name, \Closure $getEnv) | |||
| return str_getcsv($env); | |||
| } | |||
|
|
|||
| if ('nullable' === $prefix) { | |||
| return '' === $env ? null : $env; | |||
There was a problem hiding this comment.
What if the env var is not defined at all? To me, not providing the env var means null.
Currently, you can already get a null behavior for FOO env var by setting a env(FOO) parameter:
parameters:
env(FOO): nullso null is used as default value if FOO env var is not set.
Don't we just need this allowing an env var not to be set to get null, without defining a parameter as default?
There was a problem hiding this comment.
i think we should avoid
env(DEFAULT_NULL): null
%env(DEFAULT_NULL)%
competing with
%env(nullable:RUNTIME_ARBITRARY_NULL)%
to take this further we could also deprecate setting null defaults, in favor of always using nullable: + string values (related to #27808)
There was a problem hiding this comment.
Yes, my point is %env(nullable:FOO)% should be accepted even if FOO is not set and no env(FOO) parameter exists => injected value will be null.
Second point: not sure we need the empty string to be considered null.
There was a problem hiding this comment.
My worry is RUNTIME_ENV= can only be nulled by removing it or move it to config.
Currently there's only one source where null can come from, and it's: env(DEFAULT): ~
Adding nullable creates 2 scenarios. Hence my suggestion to deprecate env(DEFAULT): ~ in favor of
RUNTIME=
env(DEFAULT): ""
%env(nullable:DEFAULT)%
%env(nullable:RUNTIME)%
IMHO that's the easiest to reason about.
There was a problem hiding this comment.
When the env var is not defined, we already added the "default" processor in #28976. I think this processor should do one thing, turn empty strings to null - what it does currently - and "default" covers the other use cases.
I also don't agree deprecating env(FOO): ~ is a good idea: we shouldn't deprecate for the sake of it.
There was a problem hiding this comment.
Note that "default" has not been released yet so we can also alter its behavior.
But each processor should have one responsibility - we shouldn't have many processors doing the same in slightly different ways.
|
please rebase |
|
@nicolas-grekas Done. |
|
Sure. |
|
Thank you @bpolaszek. |
This PR was merged into the 4.3-dev branch.
Discussion
----------
Nullable environment variable processor
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets |
| License | MIT
| Doc PR | todo
Hey there,
Because environment variables are strings by design, empty environment variables are evaluated to `""` by default.
In the same way, `MY_ENV_VAR=null` will be evaluated to `"null"`, as a string.
What I suggest is to allow some environment variables to be evaluated to `null` (the real one) when their strings are _blank_ or equal _null_, _Null_ or _NULL_.
This can be easily done through a new `nullable` processor:
```bash
# .env
API_KEY=
```
```yaml
# config/services.yaml
services:
FooService:
arguments:
$apiKey: %env(nullable:API_KEY)%
```
```php
# src/Services/FooService
namespace App\Services;
final class FooService
{
/**
* @var string|null
*/
private $apiKey;
/**
* FooService constructor.
*/
public function __construct(?string $apiKey)
{
$this->apiKey = $apiKey;
}
public function doSomething()
{
// Free plan
if (null === $this->apiKey) {
// ...
}
}
}
```
That's an example that comes to my mind but there can be many others.
This can also help in using null coalesce operators in constructors instead of checking if a string equals `""` (which is very PHP4 style).
Of course it can be used in conjunction with other types, i.e. `%env(float:nullable:SOME_OPTIONAL_FLOAT_ENV_VAR)%`.
What do you think?
Commits
-------
3a604ac Nullable environment variable processor
… "default" one (nicolas-grekas) This PR was merged into the 4.3-dev branch. Discussion ---------- [DI] replace "nullable" env processor by improving the "default" one | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Neither `nullable` nor `default` are released yet. I propose to replace the `nullable` processor (see #29767) with an improved `default` one (from #28976). `%env(default::FOO)%` now defaults to `null` when the env var doesn't exist or compares to false". ping @jderusse @bpolaszek Commits ------- c50aad2 [DI] replace "nullable" env processor by improving the "default" one
Hey there,
Because environment variables are strings by design, empty environment variables are evaluated to
""by default.In the same way,
MY_ENV_VAR=nullwill be evaluated to"null", as a string.What I suggest is to allow some environment variables to be evaluated to
null(the real one) when their strings are blank or equal null, Null or NULL.This can be easily done through a new
nullableprocessor:# .env API_KEY=That's an example that comes to my mind but there can be many others.
This can also help in using null coalesce operators in constructors instead of checking if a string equals
""(which is very PHP4 style).Of course it can be used in conjunction with other types, i.e.
%env(float:nullable:SOME_OPTIONAL_FLOAT_ENV_VAR)%.What do you think?