-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[HttpKernel] Add KernelInterface::getShareDir(), APP_SHARE_DIR and %kernel.share_dir%
#62170
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
Conversation
…d `%kernel.share_dir%` to store application data that are shared between all front-end servers
9dba4e2 to
9011039
Compare
GromNaN
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.
I don't think that justifies a breaking change to KernelInterface.
| * Initialize `router.request_context`'s `_locale` parameter to `%kernel.default_locale%` | ||
| * Deprecate `ConfigBuilderCacheWarmer`, return PHP arrays from your config instead | ||
| * Add support for selecting the appropriate error renderer based on the `APP_RUNTIME_MODE` env var | ||
| * Add `KernelInterface::getShareDir()`, `APP_SHARE_DIR` and `%kernel.share_dir%` to store application data that are shared between all front-end servers |
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.
I don't think this pattern is common enough to add it to the KernelInterface.
| return parent::getBuildDir(); | ||
| } | ||
|
|
||
| public function getShareDir(): string |
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 should be nullable. Not all applications can provide such shared dir.
Setting a value that isn't a shared directory would be incorrect.
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.
By default, it fallbacks to cache_dir which keeps BC.
src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
Show resolved
Hide resolved
| return parent::getBuildDir(); | ||
| } | ||
|
|
||
| public function getShareDir(): string |
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.
By default, it fallbacks to cache_dir which keeps BC.
GromNaN
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.
Ok. This directory can also be used for the sqlite database
DATABASE_URL="sqlite:///%kernel.share_dir%/data/database.sqlite"
|
Thank you @nicolas-grekas. |
…ro0NL) This PR was squashed before being merged into the 7.4 branch. Discussion ---------- [FrameworkBundle] Expose share directory in AboutCommand | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | Fix #... <!-- prefix each issue number with "Fix #"; no need to create an issue if none exists, explain below --> | License | MIT Follow up of #62170 I think it's good to expose the actual fs paths. Commits ------- 40de429 [FrameworkBundle] Expose share directory in AboutCommand
|
@nicolas-grekas would you say that its fine that this new directory flysystem:
storages:
default.storage:
adapter: 'local'
options:
directory: '%kernel.share_dir%/storage/default' |
|
Yes of course - anything that fits as a shared mount point. |
|
Yes, as it's a shared filesystem. But then it should not fallback to the cache directory? |
|
Awesome, thx. In Sulu we work on the new major release. Our app caches already not following Symfony Directory structure (because of a similar issue what nicolas is solving here). So want to change it for the next major version already to the new share directory, and as we switch in the new major to the flysystem bundle I think make sense then use also that the new directory for the files. Not sure if Symfony/Flex / Recipes allow to be different for the same Bundle Version depending on different Symfony Versions if yes maybe the recipe can be changed using the new directory when Symfony 8 is used. For 7.4 its maybe to critical as its the cache dir there maybe. /cc @maxhelias |
|
@alexander-schranz As far as I know, for the same recipe version it's not possible. However, we can create a new recipe for a new minor version with a conflict on the Symfony version. See this example : https://github.com/symfony/recipes/blob/main/symfony/asset-mapper/6.4/manifest.json#L24-L28 |
…tShareDir (alexander-schranz) This PR was squashed before being merged into the 7.4 branch. Discussion ---------- [FrameworkBundle] Change HttpCache directory to use new getShareDir | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes <!-- if yes, also update src/**/CHANGELOG.md --> | Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md --> | Issues | Fix #... <!-- prefix each issue number with "Fix #"; no need to create an issue if none exists, explain below --> | License | MIT In `@sulu` we are already using for app caches and http caches a shared directory root directory the last years, because we had a 2 kernel setup (var/cache/admin, var/cache/website) where both caches are shared (var/cache/common) between the application. With Symfony also adding similar things in #62170 providing share dir for the app caches I think it make sense to include the http cache there as its also more like an app cache. Commits ------- 039c228 [FrameworkBundle] Change HttpCache directory to use new getShareDir
While deploying Symfony applications to production servers, I realized we badly miss a way to split the "app" cache pools out of the
var/cachedirectory.The reason is that Symfony apps perform at maximum performance when
var/cacheis mounted on a local disk. Yet this locality requirement isn't compatible with "app" cache pools, which have to be shared between all instances of a multi-front setup. Before using another storage than the filesystem for cache pools, it's quite common to use a shared mount for thevar/cachefolder, precisely to accommodate for app pools, at the detriment of performance for local-only artifacts.I had this feature on my todo since a few months and forgot about it. Having this in 7.4 would be great: it would provide an out-of-the-box way to best-deploy Symfony apps without touching their code.