search

saltstack-formulas / php-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
57 stars 232 forks source link

Fix missing value for php.lookup.fpm.user in multi-php mode #191

Closed philpep closed 5 years ago

philpep commented 5 years ago

This fixes the rendering issue:

[CRITICAL] Rendering SLS 'base:php.ng.fpm.config' failed: Jinja variable 'dict object' has no attribute 'user'

When using multiple php versions, this is issued by "{{ php.lookup.fpm.user }}" from php/ng/fpm/config.sls

Add default users in group in the relevant 'fpm' dict from map.jinja (this is to set owner and group to 'pool.d' directory).

myii commented 5 years ago

@philpep We're very close to merging #183, which would have an impact on this PR (and vice versa). Do you rely on this formula? Would you mind testing #183 so that we can confirm it works for users (in production)? Specifically in respect to this comment: https://github.com/saltstack-formulas/php-formula/pull/183#issuecomment-520935686.

myii commented 5 years ago

@philpep In terms of this PR, the commit message is failing the commitlint check:

✖   subject may not be empty [subject-empty]
✖   type may not be empty [type-empty]

So the commit message could be changed in the following way:

-Fix missing value for php.lookup.fpm.user in multi-php mode
+fix(map): fix missing value for php.lookup.fpm.user in multi-php mode

There's more information about the semantic-release procedure here:

philpep commented 5 years ago

Hi @myii , thanks for reviewing ! I 'updated the commit message accordingly. This issue was introduced in implementation of multi-php versions !167 , me and @arthurlogilab are working on the same code base. We'll give a try to #183 (we use several debian/centos versions).

Feel free to merge #183 before this one, I can rebase.

myii commented 5 years ago

@philpep You're welcome, thanks for resolving the commit message. Your help with #183 would be fantastic, since @sticky-note has put in a lot of effort and done a great job in preparing that PR (and all of the PRs leading up to it). Maybe we can merge this one first; let's ask on that PR to see what the response is.

myii commented 5 years ago

Since #192 got merged, we have to rebase #183 anyway, so I've merged this as well. Thanks, @philpep.

saltstack-formulas-travis commented 5 years ago

:tada: This PR is included in version 0.40.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: