Feat: Add environment variable support for DSN #68
Conversation
| @@ -8,7 +8,7 @@ | |||
| "symfony/http-client": "^5.0 | ^6.0 | ^7.0", | |||
| "symfony/cache": "^5.0 | ^6.0 | ^7.0", | |||
| "nyholm/psr7": "^1.0", | |||
| "unleash/client": "^1.6 | ^2.0", | |||
| "unleash/client": "^2.4", | |||
|
|
||
| use Stringable; | ||
|
|
||
| final readonly class LateBoundDsnParameter implements Stringable |
There was a problem hiding this comment.
@RikudouSage @sighphyre this seems to me to be the wrong abstraction.
It seems to me it assumes the env var will directly evaluate to a DSN (feel free to correct if this is wrong), but that's often not the case.
For example, the config might look like this:
unleash_client:
dsn: '%env(trim:file:UNLEASH_DSN_FILE)%'with this workflow:
UNLEASH_DSN_FILE=/run/secrets/unleash_dsn- read this file and then trim its contents using built-in env var processors
This should typically all work by default because Symfony itself does the late binding for us, it knows the variable needs to be re-evaluated at runtime before it's passed to the service.
This currently doesn't happen to me, the env var evaluates to an empty string.
For comparison, FOS Elastica bundle supports this same process like so:
fos_elastica:
clients:
default:
url: '%env(trim:file:SEARCH_DSN_FILE)%'and there it works as expected.
There was a problem hiding this comment.
Can you post it as a separate issue, please?
There was a problem hiding this comment.
Sure, I'm just checking if my assumption about env var containing the DSN is correct?
There was a problem hiding this comment.
Done in #73, notice also the issue with tags getting created constantly and end up being seen as releases by Packagist: https://packagist.org/packages/unleash/symfony-client-bundle
Description
This uses the recently introduced Stringable support for basic parameters to allow runtime bound values in the container, which is the expected way to support env binding in Symfony apps (as opposed to just binding the parameter from the env variable at the time the container was built).
A proxy class for static values has been created as well so that we don't have to do another unnecessary round of if/else and just provide a service with the same name as opposed to distinguishing between the service (
Stringableinstances) and parameters (staticstringvalues), especially because they have different syntax in theservices.yamlconfig (%parameter_name%vs@service_name).Fixes #66
Type of change
How Has This Been Tested?
Checklist: