Skip to content

[DI] Unable to use env() on params passed to ReplaceAliasByActualDefinitionPass #21609

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

Closed
andrerom opened this issue Feb 14, 2017 · 15 comments
Closed

Comments

@andrerom
Copy link
Contributor

andrerom commented Feb 14, 2017

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version 3.2.x

It's stated in #20434 that env variables currently does not support being used in place for service names, which limits it's usefulness.

Is there plans to provide a best practice around this?

Possible alternatives:
A. Change how configuration of handlers are done to allow switching service on runtime Does not make sense as pointed out by @stof below, often you need to compile config depending on a handler, this is something to do at compile time and not run time
B. Introduce a compile time env variable support as well so we can also be allowed to inject variables at compile time without having to resolve to custom env.php kind of solutions.
C. If there are no plans to tackle this with additional features, improvment potential here is to improve doc and more clearly throw on use like this as done in #20687.

Example:

  [Symfony\Component\DependencyInjection\Exception\EnvParameterException]                    
  Incompatible use of dynamic environment variables "MAILER_TRANSPORT" found in parameters.  
                                                                                             

Exception trace:
 () at vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/Compiler/Compiler.php:137
(...)

                                                                                                                         
  [Symfony\Component\DependencyInjection\Exception\InvalidArgumentException]                                             
  Unable to replace alias "swiftmailer.mailer.default.transport.real" with actual definition "%env(MAILER_TRANSPORT)%".  
                                                                                                                         

Exception trace:
 () at vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/Compiler/ReplaceAliasByActualDefinitionPass.php:63
(...)
                                                                              
  [Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException]  
  You have requested a non-existent service "%env(MAILER_TRANSPORT)%".        
                                                                              

Exception trace:
 () at vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/ContainerBuilder.php:795
(...)
@stof
Copy link
Member

stof commented Feb 14, 2017

Aliases cannot use parameters at all (since Symfony 2.0.0), be there env variables or no.

And btw, supporting env variables there is a no-go IMO. The whole point of env variables is that they are resolved at runtime. And aliases are validated and resolved at compile time. Adding a dynamic target for an alias would require rewriting all code dealing with aliases (and adding overhead for all aliases in the container, to check at runtime whether they need to be resolved dynamically...)

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Feb 14, 2017

I suggest opening an issue on swiftmailer bundle instead, this is where the issue exists: too many logic done at compile time

@stof
Copy link
Member

stof commented Feb 14, 2017

@nicolas-grekas this issue comes from MonologBundle. And processing the whole bundle config at runtime to decide which handlers to create and how looks insane to me. I don't think the overhead it will add is justified, compared to being able to switch the logger handlers at runtime without a deployment (thus, most of them expect different configuration settings, and potentially different dependencies, so it would simply not work).

@nicolas-grekas
Copy link
Member

indeed :)

@stof
Copy link
Member

stof commented Feb 14, 2017

btw, the same is true for the swiftmailer transport: each transport expect different config settings

@andrerom
Copy link
Contributor Author

Well there is still option B and C available :)

@nicolas-grekas
Copy link
Member

B. Introduce a compile time env variable support as well so we can also be allowed to inject variables at compile time without having to resolve to custom env.php kind of solutions.

Why not use SYMFONY__* env vars instead of env.php? see also #20100 (thus if you think this should not be deprecated, please comment there)

@andrerom
Copy link
Contributor Author

andrerom commented Feb 14, 2017

Why not use SYMFONY__* env vars

They are rather pointless, they are overridden by defined parameters making them useless for the case at hand (yml having defaults, env being able to override), hence support use with and without.

instead of env.php

I did get rid of env.php approach, but what I ended up with for now is to use comoser.json incenteev-parameter.env-map for compile time params, and env() for run time options. But with all the recent env improvements I was hoping to avoid having to use incenteev/composer-parameter-handler, I was kind of expecting some standard convention could solve this now ;)

@nicolas-grekas
Copy link
Member

OK. You can also do that in your bundle extension: resolve env vars and inject them in params. That'd be 100% compile time.

@andrerom
Copy link
Contributor Author

true, but if symfony came with something like %compile_env()% then I would not have to hard code that in the bundle, it can be user defined in the yml files we provide to end user.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Apr 17, 2017

I'd be in favor of closing this one, and replacing it with a doc issue. IMHO, we should not provide helpers to make people use env vars at compile time. Doing so means making the container not runtime-configurable for the corresponding envs. While there are legitimate use cases to do so, there are already ways to do that (the simplest one being to getenv() in a compiler pass, the more advanced being to use ContainerBuilder::resolveEnvPlaceholders()).

@andrerom
Copy link
Contributor Author

ok

@cordoval
Copy link
Contributor

is there a way to shut down the %env()% trick? honestly i ran into so many issues with it that i just use a parameters.php to do set my parameters, wasted a lot of time because of this.

@javiereguiluz
Copy link
Member

@cordoval it's not really a trick but a feature :) And there's no direct way to disable it ... but there's an indirect way: not using it in any config file.

@cordoval
Copy link
Contributor

thanks @javiereguiluz i know the features are with good intentions, just so frustrating 👴 uff uff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants