[DI] ContainerBuilder::compile() can optionally resolve env vars in parameter bag #21460

nicolas-grekas · 2017-01-30T08:47:12Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	yes
Fixed tickets	#21420
License	MIT
Doc PR	-

Here is a new feature allowing one to do $container->compile(true) and get the env vars resolved using the current env.

chalasr · 2017-01-30T10:18:52Z

👍

olvlvl · 2017-01-30T11:37:26Z

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

@@ -555,15 +556,27 @@ public function prependExtensionConfig($name, array $config)
     *  * The parameter bag is frozen;
     *  * Extension loading is disabled.
     */
-    public function compile()
+    public function compile(/* $resolveEnvPlaceholders = false */)


How about using a flag (e.g. self::RESOLVE_ENV_PLACEHOLDERS) instead of a boolean, so that additional features may be introduced without requiring additional parameters?

IMHO, we shouldn't plan for additional unplanned features :) This can be changed easily in the future if needed.

javiereguiluz · 2017-01-30T11:55:37Z

src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

+        $_ENV['DUMMY_ENV_VAR'] = 'du%%y';
+
+        $container = new ContainerBuilder();
+        $container->setParameter('bar', '%% %env(DUMMY_ENV_VAR)%');


What will happen in the other two cases: 1) undefined env var; 2) undefined env var, but default value defined for it. Should we add those cases to this test too?

same as usual, this is exactly the same code/logic

tests added

fabpot · 2017-01-30T19:29:19Z

Can you add some docs in the phpdocs?

nicolas-grekas · 2017-01-30T20:37:53Z

phpdoc added

fabpot · 2017-02-01T00:04:59Z

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

@@ -555,16 +556,37 @@ public function prependExtensionConfig($name, array $config)
     *  * Parameter values are resolved;
     *  * The parameter bag is frozen;
     *  * Extension loading is disabled.
+     *
+     * @param bool $resolveEnvPlaceholders Whether %env()% parameters should be resolved using the current
+     *                                     env vars or be replaced by uniquely identifiable placeholders


That describes what it does, but I'd like to understand also when I should pass true here.

comment updated

…arameter bag

fabpot · 2017-02-09T17:52:17Z

Thank you @nicolas-grekas.

…e env vars in parameter bag (nicolas-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] ContainerBuilder::compile() can optionally resolve env vars in parameter bag | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #21420 | License | MIT | Doc PR | - Here is a new feature allowing one to do `$container->compile(true)` and get the env vars resolved using the current env. Commits ------- a3fd512 [DI] ContainerBuilder::compile() can optionally resolve env vars in parameter bag

zippy1981 · 2017-05-18T14:57:18Z

I tried to capture how to do this in a stackoverflow answer.

pwm · 2017-06-15T15:04:27Z

@zippy1981 Thanks for that SO answer, however I can't seem to get it working. If I replace $container->compile(); with $container->compile(true); in the initializeContainer() method of Symfony\Component\HttpKernel\Kernel then everything works fine, however if I add $container->compile(true); to my own bundle then nothing happens. Where exactly should I add this line?

stof · 2017-06-15T16:17:52Z

@pwm why would you want to use this argument in your bundle ?

pwm · 2017-06-16T09:52:37Z

@stof I'm trying to get env vars work as monolog config values, pretty much exactly like the SO entry above, but I get an Incompatible use of dynamic environment variables... error. If I understand it correctly this PR is resolving this issue.

carsonbot added Status: Needs Review DependencyInjection Feature labels Jan 30, 2017

nicolas-grekas mentioned this pull request Jan 30, 2017

[DI] Unable to resolve env() #21420

Closed

nicolas-grekas added this to the 3.3 milestone Jan 30, 2017

olvlvl reviewed Jan 30, 2017

View reviewed changes

javiereguiluz reviewed Jan 30, 2017

View reviewed changes

nicolas-grekas force-pushed the di-compile-env branch from 8b278b2 to 13ea687 Compare January 30, 2017 16:37

nicolas-grekas force-pushed the di-compile-env branch from 13ea687 to ff4c1f0 Compare January 30, 2017 20:37

nicolas-grekas force-pushed the di-compile-env branch from ff4c1f0 to 1a2b1cb Compare January 31, 2017 12:20

fabpot reviewed Feb 1, 2017

View reviewed changes

nicolas-grekas force-pushed the di-compile-env branch 3 times, most recently from a735b0e to 6ea3a24 Compare February 3, 2017 12:26

[DI] ContainerBuilder::compile() can optionally resolve env vars in p…

a3fd512

…arameter bag

nicolas-grekas force-pushed the di-compile-env branch from 6ea3a24 to a3fd512 Compare February 9, 2017 15:17

fabpot merged commit a3fd512 into symfony:master Feb 9, 2017

nicolas-grekas deleted the di-compile-env branch February 14, 2017 16:21

fabpot mentioned this pull request May 1, 2017

Release v3.3.0-BETA1 #22603

Merged

drzraf mentioned this pull request Jun 9, 2017

issue #982: support environment variable inside behat.yml Behat/Behat#1048

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DI] ContainerBuilder::compile() can optionally resolve env vars in parameter bag #21460

[DI] ContainerBuilder::compile() can optionally resolve env vars in parameter bag #21460

nicolas-grekas commented Jan 30, 2017 •

edited

Loading

chalasr commented Jan 30, 2017 •

edited

Loading

olvlvl Jan 30, 2017 •

edited

Loading

nicolas-grekas Jan 30, 2017

javiereguiluz Jan 30, 2017

nicolas-grekas Jan 30, 2017

nicolas-grekas Jan 30, 2017

fabpot commented Jan 30, 2017

nicolas-grekas commented Jan 30, 2017

fabpot Feb 1, 2017

nicolas-grekas Feb 9, 2017

fabpot commented Feb 9, 2017

zippy1981 commented May 18, 2017

pwm commented Jun 15, 2017

stof commented Jun 15, 2017

pwm commented Jun 16, 2017

[DI] ContainerBuilder::compile() can optionally resolve env vars in parameter bag #21460

[DI] ContainerBuilder::compile() can optionally resolve env vars in parameter bag #21460

Conversation

nicolas-grekas commented Jan 30, 2017 • edited Loading

chalasr commented Jan 30, 2017 • edited Loading

olvlvl Jan 30, 2017 • edited Loading

Choose a reason for hiding this comment

nicolas-grekas Jan 30, 2017

Choose a reason for hiding this comment

javiereguiluz Jan 30, 2017

Choose a reason for hiding this comment

nicolas-grekas Jan 30, 2017

Choose a reason for hiding this comment

nicolas-grekas Jan 30, 2017

Choose a reason for hiding this comment

fabpot commented Jan 30, 2017

nicolas-grekas commented Jan 30, 2017

fabpot Feb 1, 2017

Choose a reason for hiding this comment

nicolas-grekas Feb 9, 2017

Choose a reason for hiding this comment

fabpot commented Feb 9, 2017

zippy1981 commented May 18, 2017

pwm commented Jun 15, 2017

stof commented Jun 15, 2017

pwm commented Jun 16, 2017

nicolas-grekas commented Jan 30, 2017 •

edited

Loading

chalasr commented Jan 30, 2017 •

edited

Loading

olvlvl Jan 30, 2017 •

edited

Loading