guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#28769] [PATCH] gnu: services: Add php-fpm.


From: nee
Subject: [bug#28769] [PATCH] gnu: services: Add php-fpm.
Date: Thu, 23 Nov 2017 21:11:48 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

Hello sorry about not replying for such a long time, and thank you for
reviewing my patches again.

Am 02.11.2017 um 20:17 schrieb Christopher Baines:
> 
> I've now attached a system test for php-fpm, that sets it up with
> nginx, creates a basic php file, and checks that it can be requested.

That's great thanks for saving me a bunch of work with this. ;-)

> 
> This seems to work, which I was a little surprised at, as I was
> suspecting problems with the socket permissions.
> 
> I had a look, and while the nginx workers in the test system are not
> root, the nginx master process is, so maybe that allows it to work...
I don't think that's the reason, because I remember it not working at
first when I didn't have the permissions set.

>> I renamed the default workers-log-file to php7-fpm.www.log as it is
>> usually called. The php-fpm log-file is now called php7-fpm.log.
> 
> What I think I meant to say here was, I'm not sure the php _version_ is
> adding much to the log file name (rather than "I'm not sure the php is
> adding").
> 
> The php package version is used in a few places, and while I can
> imagine this being consistent with other distributions, it does add a
> bit of complexity to the default values in the service, and I'm not
> sure what benefit it brings?
> 
If users want to run multiple php versions, they only have to change the
version in the php-package and pass that package along all the services.

My perception of the php landscape was that the major releases aren't
100% reliably backwards compatible and some applications depend on older
stable releases, so that it is not too uncommon to run multiple php
versions the same system.

Here is a quote from: https://wordpress.org/about/requirements/

"""
Why do we support older versions?

We strongly recommend the latest versions of PHP and MySQL, but we
understand that this isn’t right for everyone, and that sometimes hosts
can be slow or hesitant to upgrade their customers since upgrades to PHP
and MySQL have historically broken applications.
"""

An alternative could be to include the php-package hash in the socket
name, but I'm not sure if that would work with nginx and it's currently
missing reload when a system-reconfigure is done. Or services could be
generally more isolated somehow.

WDYT?

>>>> +  (file          php-fpm-configuration-file ;#f | file-like
>>>> +                 (default #f)))
> 
> ...
> 
>>>> +                       (service-extension account-service-type
>>>> +                                          php-fpm-accounts)))
>>>> +                (default-value (php-fpm-configuration))))  
>>>
>>> Filling in the description (a relatively new field on the service type)
>>> would be a great addition here.
>>>   
>> Ah, yes I'm mostly using the web documentation and other services from
>> web.scm as reference. Thanks for the update.
>> What would be a good value for this field? I just used "The php-fpm
>> service-type." for now.
> 
> That is ok, but it could probably be more useful. I think ideally it
> would describe more about what the service offers.
> 
> Users might encounter this when searching for services for example:
> 
> → guix system search php
> name: php-fpm
> location: gnu/services/web.scm:607:2
> extends: shepherd-root activate account
> description: The php-fpm service-type.
> relevance: 5

I ran `guix sysytem search *` and it seems most descriptions start with
'Run' or 'Provide' I changed it to:
 "Run `php-fpm' to provide a fastcgi socket for calling php through a
webserver."


> I've attached a patch containing a couple of copy+paste fixes, and a
> system test.
> 
> It would be good to get your opinion on the system test, does it test
> the right things?
> 
The tests look good to me.
I added another test that adds two numbers and checks for the result in
the response to see if php actually did something with the file.

> If you like the look of it, I'd suggest including those changes in the
> commit that adds the service, and then sending the updated patches.
> I've included a changelog in the commit message.
> 
Here are the updated patches. Thank you for your tests!

Attachment: 0001-guix-utils-add-version-major.patch
Description: Text Data

Attachment: 0002-gnu-services-Add-php-fpm.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]