[FrameworkBundle] added new parameter router.request_context.url #35580
Conversation
The router.request_context.url can be used instead of router.request_context / router.request_context.host etc to define url in console commands
...Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/RequestContextPassTest.php
Outdated
Show resolved
Hide resolved
...Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/RequestContextPassTest.php
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/RequestContextPass.php
Outdated
Show resolved
Hide resolved
| @@ -489,6 +489,7 @@ private function addRouterSection(ArrayNodeDefinition $rootNode) | |||
| ->scalarNode('host')->defaultValue('%router.request_context.host%')->end() | |||
| ->scalarNode('scheme')->defaultValue('%router.request_context.scheme%')->end() | |||
| ->scalarNode('base_url')->defaultValue('%router.request_context.base_url%')->end() | |||
| ->scalarNode('url')->defaultValue('%router.request_context.url%')->end() | |||
There was a problem hiding this comment.
Should we add a config validation to check if this or others are filled but not both? To avoid « conflict »?
Also what about deprecating others if it does the same?
There was a problem hiding this comment.
It's a good question.
In this PR url syntax will override "standard" one. Validation may be useful too.
I think, that deprication is not needed, becouse old syntax can be useful. For example, user can pass only base_url.
|
didnt we solve it with #35281 already? Im not sure introducing more params is for the better, as we moved to semantic config. Is the |
|
@ro0NL, url node is not prefered, but it useful for some developers I see. Configure of host / sheme / base url is fine, and url is just a new way todo it. Shortcut in a way. |
There was a problem hiding this comment.
Thanks for working on this.
I would suggest two non-trivial changes here:
- we should support only the new "base_uri" option in the config - let's drop the 3 others
- the parameter should be read at runtime, to make it compatible with env vars. This means it cannot be processed in a compiler pass, but where the value is needed at runtime.
Up to give it a try?
|
@nicolas-grekas, thanks for you review. Do you have any idea, how to make it compatible with env vars? I understand, way with comliler pass, but how I can do that with envs? Can you give me a more detailed way? |
|
and by the way ... if I drop 3 others options... I will be BC break isn't it? |
|
The options are new to master so they can be dropped if I recall correctly. |
|
Ok, will try. Thanks a lot. |
|
Friendly ping @pyatnitsev |
|
Thank you for ping. I'll try to do this on this weekend. |
|
Thank for opening @pyatnitsev, I took over in #36651 |
…RI with a DSN (nicolas-grekas) This PR was merged into the 5.1-dev branch. Discussion ---------- [FrameworkBundle] Allow configuring the default base URI with a DSN | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fixes #35121, replaces #35580, partially reverts #35281 | License | MIT | Doc PR | - Instead of defining 3-4 parameters, this PR enables using a single DSN to configure the default URL context (for commands mainly): ``` framework: router: base_uri: 'https://my.host:8443/base-path/' ``` When using parameters directly, one can now set the same absolute URI in the `router.request_context.base_url` parameter, this will provide the same benefit. Commits ------- 250fa7e [FrameworkBundle] Allow configuring the default base URI with a DSN
The router.request_context.url can be used instead of router.request_context / router.request_context.host etc to define url in console commands