[DependencyInjection] Resolve parameters in tag arguments #37243
Conversation
|
I'm fine with the implementation but I'm not sure I understand the use case (I just re-read the linked issue but that's doesn't help to get when this is useful). Could you please elaborate a bit on this aspect? (rebase needed btw) |
|
We have a multi-tenant setup, where each of the tenants have their own URL and those URLs are used internally to identify them. Since we have multiple environments (dev, staging, prod) these URLs change per environment, so we're storing those in parameters and override them in different environments (eg dev.symfony.com and www.symfony.com). What we want to do is be able to tag certain services, like CRM clients, with the URL of the tenant so that we can use a locator to get the CRM client of the current client, based on the current URL. For that we need Symfony to be able to process parameters in tag values. So right now we have a class that takes a ServiceLocator and the ParameterBag as arguments and then does the replacement internally, and that works, but it's rather ugly. As an aside, we don't use DotEnv, just plain old parameters. |
d352ff0 to
24d4e79
Compare
|
Rebase onto master done @nicolas-grekas |
24d4e79 to
3dba1fe
Compare
|
Thank you @rpkamp. |
|
This is a BC break, because it now requires escaping |
|
And it is a BC break without any easy upgrade path for the ecosystem (escaping the And this implementation is also built in a way which might require the whole ecosystem to adapt: parameters are resolved when using |
|
About the potential BC break, I think we can assume nobody is using a |
|
Or should this be opt-in, via a new argument to the method? |
|
I'm not sure my niche problem warrants so much effort to get it into Symfony - energy vs. gain feels off. Could you maybe revert this MR @nicolas-grekas and I can go back to the drawing board? Might end up writing a simple compiler pass inside our project instead. If that works out nicely I might create a new PR with that, which can then be optionally enabled if people wanted to. |
|
Ok to revert of course. Can you open a PR please ? I'm afk. |
Done, #37793 |
…g arguments" (rpkamp) This PR was merged into the 5.2-dev branch. Discussion ---------- Revert "[DependencyInjection] Resolve parameters in tag arguments" | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | #35337 | License | MIT | Doc PR | N/A Revert of earlier PR, see discussion #37243 Commits ------- 9731451 Revert "[DependencyInjection] Resolve parameters in tag arguments"
Arguably this could be a BC break if people are actively relying on parameters not being resolved in tag parameters, although I can't come up with a sensible use case for that scenario.