using env var fails in framework.translator.paths (cache) #24270

mathroc · 2017-09-20T16:02:43Z

Q	A
Bug report?	yes
Feature request?	no
BC Break report?	no
RFC?	no
Symfony version	3.3.9

when using an env var inside the value of a framework.translator.paths, the env var value is not resolved, and instead its placeholder is given to the translator

parameters:
  active_site: '%env(ACTIVE_SITE)%'

framework:
    translator:
        paths:
            - "%kernel.root_dir%/../src/ST/AppBundle/Resources/translations_sites/%active_site%"

UnexpectedValueException
/var/www/app/../src/ST/AppBundle/Resources/translations_sites/env_ACTIVE_SITE_1210a6ec49918eebfffc01fc4f72ca65 defined in translator.paths does not exist or is not a directory

note: same thing happen if I use the env directly like that:

framework:
    translator:
        paths:
            - "%kernel.root_dir%/../src/ST/AppBundle/Resources/translations_sites/%env(ACTIVE_SITE)%"

ro0NL · 2017-09-20T18:41:39Z

Yep, we're working on it :) see #23888

What's happening here is crucial; with this config you opt-in to dynamic values, fetched during runtime. Their real current value is not compiled into the container.

So imagine in your case the application runs with

$_ENV['ACTIVE_SITE'] = 'env_ACTIVE_SITE_1210a6ec49918eebfffc01fc4f72ca65';

(or any other value really). Given that, the error starts making sense. Basically we cant assume what the real value is, this remains even with #23888. But feedback is welcome very much :)

So to make this truly work config needs lax validation.

mathroc · 2017-09-21T02:06:36Z

thx @ro0NL

I kinda understand why I have env_ACTIVE_SITE_1210a6ec49918eebfffc01fc4f72ca65 inside the string, my guess is that env vars are not extrapolated until runtime and instead placeholder are placed inside the compiled container. Since the translator paths are used to build the translation cache, I figure this happen before the env vars are fetched so the build of the cache still have the placeholders.

So I understand that in this case Symfony would not pick up if the ACTIVE_SITE var changed once the cache is built and I’m fine with this (but I guess this can be source of confusion too)

my use case is that when I’m deploying my app I want to build the cache once and keep it for later, the env var is already set when composer runs clear:cache so I expected %env(ACTIVE_SITE)% to get its value.

for now I’m using incenteev/dynamic-parameters-bundle instead but I’d hope to get rid of it eventually as it’s unmaintained and #23901 solves my others use cases that I’m using incenteev/dynamic-parameters-bundle for

but I’m not sure what will change with #23888, it seems to change the behavior of the container compilation, so I don’t know what to expect because the container compilation seems fine to me. here it fails on cache warmup. maybe env vars are not supposed to be used at that point, in that case an error message saying so should be added. or env vars can be used when warming up the cache and . maybe it should explicitly be declared in the config to avoid confusion:

framework:
    translator:
        paths:
            - "%this_value_can_be_saved_to_cache(%kernel.root_dir%/../src/ST/AppBundle/Resources/translations_sites/%active_site%)%"

I hope I’m not completely off the mark here and that I didn’t misunderstood what is achieved by #23888

ro0NL · 2017-09-21T05:28:47Z

but I’m not sure what will change with #23888, it seems to change the behavior of the container compilation, so I don’t know what to expect because the container compilation seems fine to me

It only affects config validation. The trick is Config is aware of the placeholder values, so when encountering one we mock it with "might be value", to satisfy any checks by config. After that it returns the original placeholder value to be compiled as usual (thus in an extension $config['framework']['translator']['paths'][0] still contains the placeholder).

maybe it should explicitly be declared in the config to avoid confusion

interesting :)

My first thougt was a FileNode in config, which you can set with %env(file:ENV)%. But more or less the same approach, it would bypass file validation for envs.

Also see #23901 for reference to this file:ENV prefix.

cc @nicolas-grekas

nicolas-grekas · 2017-09-21T05:47:33Z

The exception message is buggy, but the behavior is no 8000 t a bug: not all config options are compatible with env vars. Some could be made, but until they are on a case by case basis, there should be an exception saying "hum sorry, not possible here."

In the catch block of Compiler.php, the env placeholder should be replaced by the original "%env(ACTIVE_SITE)%" one, and then, the UnexpectedValueException should have been replaced by an EnvParameterException saying basically "Incompatible use of dynamic environment variables...".

Yet, the reason why this does not apply is because the placeholder is not correctly tracked on failures.
This should be fixed in MergeExtensionConfigurationPass, which is the place playing with them.

Then, and only then, we could dive into what @ro0NL explains. We should not ignore the original issue and jump on fixing the specific case of why the env var is not allowed here. Not all config options make sense with env vars.

nicolas-grekas · 2017-09-21T05:56:29Z

this_value_can_be_saved_to_cache

that could be a nice idea in fact. It's already possible on the DI extension side, calling $container->resolveEnvPlaceholders(), which happens at compilation time and can thus inline the value of env vars found then. Yet, should we allow env vars to be by-themselves compile-time-inlineable? Something like "%env(compile:FOO)%"?

mathroc · 2017-09-21T08:56:24Z

%env(compile:FOO)% seems a bit odd, afaict, the other processors are acting on (transforming) the env var value, here it’s acting on the placeholder. so it would have to be clearly documented that this one is not like the others: it’s a flag, not something you could do by implementing your own EnvProviderInterface

mathroc · 2017-09-21T09:14:27Z

I just checked something, var_dump($container->getParameterBag()->getEnvPlaceholders()); is an empty array in the catch block of Compiler.php

mathroc · 2017-09-21T13:05:12Z

@nicolas-grekas maybe %compile:env(FOO)% instead ? this looks clearer to me

Bukashk0zzz · 2017-10-30T12:26:03Z

Any news about this issue? Are there any plan to add ability of using env variables in compile time?

nicolas-grekas · 2017-11-26T17:57:06Z

The placeholder found in the exception message is a bug, which is fixed in #25163
But using an env var in this specific option is not fixed and should not be IMHO: env vars should not be used during warmups. Use parameters instead.

nicolas-grekas · 2017-11-26T18:09:40Z

Are there any plan to add ability of using env variables in compile time?

There aren't any plans, never: everything is just contributions coming, each contributor having its own agenda and motivation. If this is a priority for you, it's up to you to make it happen :)

xabbuh · 2017-11-26T19:35:56Z

I'd say let's close here. The paths configured here are evaluated when the cache is warmed up to allow the translator to build the catalogue of translation messages. Evaluating environment variables at that time will probably add confusion when one notices that changing the variable letter does not change anything in the translator.

This PR was merged into the 3.3 branch. Discussion ---------- [DI] Fix tracking of env vars in exceptions | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24270, #24368 | License | MIT | Doc PR | - Fixes the bad exception message reported in the linked issue (regression introduced in v3.3.7). Best reviewed [ignoring whitespaces](https://github.com/symfony/symfony/pull/25163/files?w=1). Commits ------- 73f1ac5 [DI] Fix tracking of env vars in exceptions

mathroc · 2017-11-27T10:38:44Z

@nicolas-grekas the error message is much better, thank you. I’m going to open another issue to discuss how to use env var at compile time

mathroc changed the title ~~parameters using env var fails in framework.translator.paths~~ using env var fails in framework.translator.paths Sep 20, 2017

mathroc changed the title ~~using env var fails in framework.translator.paths~~ using env var fails in framework.translator.paths (cache) Sep 21, 2017

javiereguiluz added Bug DependencyInjection FrameworkBundle labels Oct 8, 2017

carsonbot added the Status: Needs Review label Oct 8, 2017

nicolas-grekas mentioned this issue Nov 26, 2017

[DI] Fix tracking of env vars in exceptions #25163

Merged

nicolas-grekas mentioned this issue Nov 26, 2017

%env(VAR)% does not work with symfony/monolog-bridge #24368

Closed

nicolas-grekas closed this as completed Nov 27, 2017

mathroc mentioned this issue Nov 27, 2017

Using environment variables at compile time #25173

Closed

xabbuh mentioned this issue Oct 22, 2018

Translator path with env var #28949

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

using env var fails in framework.translator.paths (cache) #24270

using env var fails in framework.translator.paths (cache) #24270

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

using env var fails in framework.translator.paths (cache) #24270

using env var fails in framework.translator.paths (cache) #24270

Comments

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!