uwsgi: allow custom uwsgi_param directives #926
Conversation
| should contain_concat__fragment('vhost1-500-' + Digest::MD5.hexdigest(params[:location].to_s)). | ||
| without_content(%r{uwsgi_param\s+CUSTOM_PARAM\s+.+?;}). | ||
| without_content(%r{uwsgi_param\s+CUSTOM_PARAM2\s+.+?;}) | ||
| end |
There was a problem hiding this comment.
This all looks pretty good to me. Thanks!
Assuming there shouldn't be any uwsgi_param lines in the file I'm wondering if maybe this should just be simplified to a single without_content(%r{uwsgi_param})?
There was a problem hiding this comment.
I'm sorry, but i didn't got what file you mean and what exact section?
If you mean when uwsgi_param is not set should lead to empty params - then probably it's also needed to change fastcgi section where i copied it from?
There was a problem hiding this comment.
Either is technically a test that will pass, but, assuming there shouldn't be any uwsgi_param directives in the config fragment generated by the test case (which I think is the case), the simpler test is also a better one.
Yeah, probably same thing with the example you copied from... this module has a ton of cruft and sub-optimal tests. I'm Ok with merging this either way, but if you don't mind, can you test whether the simpler case passes tests?
There was a problem hiding this comment.
Checks fail with this change. Or maybe i changed it in a wrong way? Can you take a look?
There was a problem hiding this comment.
Ah, yeah, I see...
the file still has:
include /etc/nginx/uwsgi_params with default params.
I think the attached patch (should apply cleanly to your branch) is what you want:
vhost_patch.txt
That is
- Get rid of the comment test; yes, that comment isn't there, but you're also not setting a comment, so it's not actually testing anything.
- this pattern without_content(%r{^\s+uwsgi_param\s+})
darken99
force-pushed
the
uwsgi_param
branch
4 times, most recently
from
61695e4 to
db72ceb
Compare
|
@wyardley You attached wrong patch file :) But i got the idea from your comments and it seems now everything is fine. |
|
@darken99: d'oh.... glad you got the idea, though. Merged. |
uwsgi: allow custom uwsgi_param directives
uwsgi: allow custom uwsgi_param directives
This patch allows custom uwsgi params to be added with uwsgi_param, same way fastcgi_param work.
@wyardley, It is a replacement for #767