Skip to content

Conversation

@tiengo
Copy link
Contributor

@tiengo tiengo commented Aug 30, 2013

Added parameter to create www_root directory and set permissions, disabled as default. Fastcgi template can be used to enable php file processing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be saner to set the owner and group to root as default?

@tiengo
Copy link
Contributor Author

tiengo commented Sep 12, 2013

user/group set to root as default, using nginx::config_file_owner and config_file_group, just like in apache::virtual host.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is fastcgi a modules ALWAYS used in nginx?
If not I'd not include it by default (also on existing vhosts).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will be included only in vhost that $fastcgi = present.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, waiting for the fix before merging, then.
Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By default, $fastcgi = absent... this concat fragment won't be created to all vhosts.
Not doing this right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, you are right sorry.
I didn't notice the ensure => $fastcgi,
Thank you

alvagante added a commit that referenced this pull request Sep 14, 2013
@alvagante alvagante merged commit 19b851b into netmanagers:master Sep 14, 2013
@alvagante
Copy link
Contributor

Note that I've had to merge this PR with an updated master.
Please verify if everything is working as expected
Notifying also @javierbertoli and @pafei for reference.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants