Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support pass-header option in apache::fastcgi::server #1370

Merged
merged 3 commits into from
Feb 29, 2016
Merged

support pass-header option in apache::fastcgi::server #1370

merged 3 commits into from
Feb 29, 2016

Conversation

janschumann
Copy link
Contributor

No description provided.

@@ -1,3 +1,4 @@
FastCGIExternalServer <%= @faux_path %> -idle-timeout <%= @timeout %> <%= if @flush then '-flush' end %> -host <%= @host %>
FastCGIExternalServer <%= @faux_path %> -idle-timeout <%= @timeout %> <%= if @flush then '-flush' end %> -host <%= @host -%>
<%- if @pass_header -%> -pass-header <%= @pass_header %><% end %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you actually change these two -%>\n<%- to just %> <% so they're on the same line? It seems a little confusing to have them be on two lines if you want the output to be a single line, and flush is already setting precedent for single-line code. Thanks!

@janschumann
Copy link
Contributor Author

Yes it is a bin confusing. But isn´t the readability of the template also a criteria?

Please see #1368 .. This line might get even longer.

Thoughts?

@hunner
Copy link
Contributor

hunner commented Feb 12, 2016

Ah yeah, if there are lots of options to pass then the other option is to pre-set all the variables in a <% %> block before the line to the full string like

<%
  timeout = " -idle-timeout #{@timeout}"
  host = " -host #{@host}"
  if @pass_header and ! @pass_header.empty?
    pass_header = " -pass-header #{@pass_header}"
  else
    pass_header = ""
  end
  #...
  options =  timeout + flush + host + pass_header
%>
FastCGIExternalServer <%= @faux_path %><%= options %>`

@janschumann
Copy link
Contributor Author

agreed. i´ll do that.

@hunner
Copy link
Contributor

hunner commented Feb 25, 2016

Sorry for the late response on this. Could you document your new parameter in the readme as well?

@janschumann
Copy link
Contributor Author

Sure.

While looking into the documentation of mod_fastcgi, I realized that there are some more options available, such as appConnTimeout and user etc.. Do you want this module to be as comprehensive as possible, or do you tend to implement options, when they are needed/requested.

Let me know, if you want me to implement the remaining options.

@hunner
Copy link
Contributor

hunner commented Feb 29, 2016

It's okay as-is; I always say to write for future extensibility, but present simplicity :).

👍

hunner added a commit that referenced this pull request Feb 29, 2016
support pass-header option in apache::fastcgi::server
@hunner hunner merged commit 2dcfd13 into puppetlabs:master Feb 29, 2016
@janschumann janschumann deleted the support_pass_header branch March 1, 2016 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants