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 socket communication option in apache::fastcgi::server #1368

Merged
merged 2 commits into from
Mar 1, 2016
Merged

Support socket communication option in apache::fastcgi::server #1368

merged 2 commits into from
Mar 1, 2016

Conversation

janschumann
Copy link
Contributor

For local fcgi servers it makes sense to skip the TCP stack and communicate via socket.

This is also sugested in #MODULES-1405.

Instead of introducing a new parameter socket, validate functions are used to decide if we have a host or a socket in parameter host.

It might be confusing to pass a socket path to a parameter named host, but changing the host parameter name to something like host_or_socket would introduce backwards incompatibility.

@@ -10,6 +10,14 @@

Apache::Mod['fastcgi'] -> Apache::Fastcgi::Server[$title]

if is_absolute_path($host) {
$is_host = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this would be more accurately $is_socket?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even ditch the whole $is_whatever and go for:

if is_absolute_path($host) {
  $socket = $host
} else {
  $split = split($host, ':')
  validate_ip_address($split[0])
  $_host = $host
}

Then

<%= if @_host then " -host" end %><%= if @socket then " -socket" end %>

which leaves room for other types. Are there types other than host or socket?

@tphoney
Copy link
Contributor

tphoney commented Feb 24, 2016

@janschumann is there anything else we can help with ?

@janschumann
Copy link
Contributor Author

@tphoney No thanks, but sorry for the delay. I am waiting until #1370 has been merged.

@janschumann
Copy link
Contributor Author

@hunner: afaik there are only tcp and socket connection options available ...

@janschumann
Copy link
Contributor Author

@hunner rebased and squashed

@hunner
Copy link
Contributor

hunner commented Mar 1, 2016

@janschumann Thanks. Could you also mention the new functionality in https://github.com/puppetlabs/puppetlabs-apache#host-1 ?

a socket path can now be passed to apache::fastcgi::server::host to support socket communication
@janschumann
Copy link
Contributor Author

@hunner docs added. I also removed the ip validation of the host parameter, as it may also be a hostname :-)

@hunner
Copy link
Contributor

hunner commented Mar 1, 2016

Good point.

hunner added a commit that referenced this pull request Mar 1, 2016
Support socket communication option in apache::fastcgi::server
@hunner hunner merged commit 0fca822 into puppetlabs:master Mar 1, 2016
@janschumann
Copy link
Contributor Author

@hunner thanks for merging!

@janschumann janschumann deleted the fcgi_server_support_socket branch March 1, 2016 01:25
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.

4 participants