Skip to content

[FrameworkBundle] Fix warming up routes #57554

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.

Sign up for GitHub

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

Merged
nicolas-grekas merged 1 commit into from
Jun 28, 2024
Merged

[FrameworkBundle] Fix warming up routes #57554

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function getRouteCollection(): RouteCollection
$this->collection->addResource(new ContainerParametersResource($this->collectedParameters));

try {
$containerFile = ($this->paramFetcher)('kernel.cache_dir').'/'.($this->paramFetcher)('kernel.container_class').'.php';
$containerFile = ($this->paramFetcher)('kernel.build_dir').'/'.($this->paramFetcher)('kernel.container_class').'.php';
if (file_exists($containerFile)) {
$this->collection->addResource(new FileResource($containerFile));
} else {
Expand All @@ -84,14 +84,12 @@ public function getRouteCollection(): RouteCollection

public function warmUp(string $cacheDir, ?string $buildDir = null): array
{
if (!$buildDir) {
return [];
if (null === $currentDir = $this->getOption('cache_dir')) {
Copy link
Contributor

@okhoshi okhoshi Jun 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the CacheWarmer not registered when the caching is disabled for the router ?

And with this change, the Router cache is generated once at build-time and once again if you run cache:warmup, right ?
If you run cache:warmup right after your deploy (thus, on a read-only buildDir), this will either fail or you regenerate the router cache in the cache_dir, which won't be used at all.

Copy link
Member Author

@nicolas-grekas nicolas-grekas Jun 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no it's not disabled if you set the router.cache_dir to null (which is the only way since the deprecation of the config option)

Copy link
Member Author

@nicolas-grekas nicolas-grekas Jun 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you run cache:warmup right after your deploy (thus, on a read-only buildDir), this will either fail or you regenerate the router cache in the cache_dir, which won't be used at all.

then, $buildDir will be null so that RouterCacheWarmer won't call this warmup method

Copy link
Contributor

@okhoshi okhoshi Jun 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then, $buildDir will be null so that RouterCacheWarmer won't call this warmup method

Ah right, I got caught by the double warming layer of the Router... 👍

return []; // skip warmUp when router doesn't use cache
}

$currentDir = $this->getOption('cache_dir');

// force cache generation in build_dir
$this->setOption('cache_dir', $buildDir);
// force cache generation
$this->setOption('cache_dir', $buildDir ?? $cacheDir);
$this->getMatcher();
$this->getGenerator();

Expand Down