Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] allow enabling the HTTP cache using semantic configuration#37351
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
javiereguiluz commentedJun 19, 2020
This looks nice 👍 Thanks for the config example in the description. This will help the Doc team when this feature has to be documented. |
4771a74 to82bf885Compare
yceruto left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nice idea, thanks !
c0d4bb5 to36fa034Comparenicolas-grekas commentedJun 20, 2020
PR is ready. |
maxhelias commentedJun 20, 2020
Nice! I love the idea 👍 |
nicolas-grekas commentedJun 21, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Since creating HttpCache outside of the framework was done for perf purpose, I did some benchmarks.
|
fabpot commentedJun 22, 2020
Thank you@nicolas-grekas. |
…ing semantic configuration (nicolas-grekas)This PR was merged into the 5.2-dev branch.Discussion----------[FrameworkBundle] allow configuring trusted proxies using semantic configuration| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| Deprecations? | -| Tickets | -| License | MIT| Doc PR | -On top of the improved DX this should provide, this PR (and#37351) will allow [removing the corresponding lines](symfony/recipes#790) from `index.php` & recipes.Using values:```yamlframework: trusted_proxies: '127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16'#or trusted_proxies: '%env(TRUSTED_PROXIES)%'````trusted_headers` is similar but is an array of headers to trust.```yamlframework: # that's the defaults already trusted_headers: ['x-forwarded-all', '!x-forwarded-host', '!x-forwarded-prefix']```Commits-------af9dd52 [FrameworkBundle] allow configuring trusted proxies using semantic configuration
| ->arrayNode('private_headers') | ||
| ->performNoDeepMerging() | ||
| ->requiresAtLeastOneElement() | ||
| ->fixXmlConfig('private_header') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this must be on the parent node to work
| ->integerNode('default_ttl')->end() | ||
| ->arrayNode('private_headers') | ||
| ->performNoDeepMerging() | ||
| ->requiresAtLeastOneElement() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why would we require at least one element here ?
…ion (nicolas-grekas)This PR was merged into the 5.x branch.Discussion----------[FrameworkBundle] fix config declaration of http_cache option| Q | A| ------------- | ---| Branch? | 5.2| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -Addresses comments from@stof in#37351 (review)Commits-------2514cf1 [FrameworkBundle] fix config declaration of http_cache option
…ion (nicolas-grekas)This PR was merged into the 5.x branch.Discussion----------[FrameworkBundle] fix config declaration of http_cache option| Q | A| ------------- | ---| Branch? | 5.2| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -Addresses comments from@stof insymfony/symfony#37351 (review)Commits-------2514cf1c1d [FrameworkBundle] fix config declaration of http_cache option
Uh oh!
There was an error while loading.Please reload this page.
Right now, using the HTTP cache requires tweaking the
public/index.phpfileas explained in the doc.This PR removes this requirement by allowing one to do this instead: