Skip to content

[HttpKernel] Enable optional cache-warmers when cache-dir != build-dir #57553

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

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Jun 27, 2024

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

We should be running optional cache warmers during kernel compilation when cache-dir != build-dir. This gives them an opportunity to run, which is missing right now.

@carsonbot carsonbot added this to the 6.4 milestone Jun 27, 2024
@nicolas-grekas nicolas-grekas changed the title [FrameworkBundle][HttpKernel] Fix behavior of cache-warmers regarding cache vs build dirs [HttpKernel] Enable optional cache-warmers when cache-dir != build-dir Jun 27, 2024
@Okhoshi
Copy link
Contributor

Okhoshi commented Jun 27, 2024

Your second approach seems better to achieve what was missing 👍

$cacheWarmer->enableOptionalWarmers();
}

$preload = array_merge($preload, (array) $cacheWarmer->warmUp($buildDir, $buildDir));
Copy link
Member

Choose a reason for hiding this comment

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

why passing the buildDir twice ?

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.

because passing the cacheDir is highly confusing (which one should I use?), and warmers don't need to know where the cache-dir is when there is a build-dir (but they need to know they're running for the buildDir, so the argument needs to be passed, thus 2)

Copy link
Member Author

Choose a reason for hiding this comment

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

I've reverted this part: passing the cacheDir actually forces warmers to opt-in for the build dir if they really want to support it
this is safer: not updated cache warmer will continue writing to the cache dir

nicolas-grekas added a commit that referenced this pull request Jun 28, 2024
This PR was merged into the 7.1 branch.

Discussion
----------

[FrameworkBundle] Fix warming up routes

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Follows #52962 and builds on the semantics of #57553.

Commits
-------

cc918e9 [FrameworkBundle] Fix warming up routes
@nicolas-grekas nicolas-grekas merged commit db6ddb2 into symfony:6.4 Jun 28, 2024
9 of 10 checks passed
@nicolas-grekas nicolas-grekas deleted the fix-warmer branch June 28, 2024 09:29
This was referenced Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants