Skip to content
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

fix: no longer automatically execute cachetool task #104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stijnbernards
Copy link
Contributor

Issue

Currently the Hypernode deploy always executes the cachetool opcache flush task.
This task is extremely flakey and fails a lot on our end. Same issue as on the Hipex servers.

Solution

This task can be safely removed, or at least marked optional as it's not required on Hypernode.

For reference see: https://deployer.org/docs/7.x/avoid-php-fpm-reloading
And in the Hypernode NGINX configuration /etc/nginx/fastcgi_params the following line fastcgi_param SCRIPT_FILENAME $realpath_root$fastcgi_script_name;
Which makes it so that no longer opcache has to be flushed.

I've tested this and validated this on multiple customers on our end.

Copy link
Collaborator

@tdgroot tdgroot left a comment

Choose a reason for hiding this comment

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

Good suggestion, I agree that this should be optional! Some of our customers are actually relying on cachetool because they manually opted for disabling opcache revalidation so let's keep the task in here, but make it optional.

@@ -35,8 +35,6 @@ class CachetoolTask extends TaskBase

public function register(): void
{
after('deploy:symlink', 'cachetool:clear:opcache');
after('cachetool:clear:opcache', 'cachetool:cleanup');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the after after('cachetool:clear:opcache' can remain, because when you actually make use of that task, you don't need to think about the cleanup.

Otherwise, I agree that this should be optional!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants