Skip to content

Commit

Permalink
Don't update the FPM config to change the home dir twice
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcameron
jcameron committed Dec 10, 2022
1 parent e430e59 commit 8c0b326
Showing 1 changed file with 0 additions and 1 deletion.
1 change: 0 additions & 1 deletion feature-web.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,6 @@ sub modify_web
[ "upload_tmp_dir", $oldd->{'home'}, $d->{'home'}, 1 ],
);
&fix_php_ini_files($d, \@fixes);
&fix_php_fpm_pool_file($d, \@fixes);
}
&release_lock_web($d);
&create_framefwd_file($d);
Expand Down

6 comments on commit 8c0b326

@iliajie
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! That seems to work. Although, I have found a tiny bug. If we have PHP logging enabled in templates, and set PHP mode to Disabled (none) first, and then re-enabled it with FPM, then PHP logging for the FPM pool is still disabled. Worth mentioning, that the same operation works just fine when switching from disabled to FCGI mode, i.e. PHP logging gets enabled, but not in FPM mode. Also, if you create a new domain and FPM is the default execution mode and PHP logging is enabled in templates then logging for the PHP pool is getting to set correctly (to enabled) but if you toggle FPM->none-FPM, then logging is not getting re-enabled, even though it's set to on in templates.

Besides (related), can we please not print the following message when disabling PHP and PHP error log is enabled? It creates noise for nothing:

image

@jcameron
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I'll fix that confusing message. Maybe I'll also make the PHP log use the default path when switching out of "none" mode, although this could be confusing to the user if they has logging disabled before switching modes originally..

@iliajie
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why don't we set it to whatever it is on the template, the same way we do it when creating a virtual server?

@jcameron
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do ..

@jcameron
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just checked in a fix for that..

@iliajie
Copy link
Collaborator

Choose a reason for hiding this comment

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

Excellent. Thanks. It seems to work just fine!

Please sign in to comment.