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

Modules-4500 Add optional "AdvertiseFrequency" directive in cluster.conf template #1601

Merged
merged 3 commits into from
Mar 10, 2017

Conversation

EmersonPrado
Copy link
Contributor

If calling class defines advertise_frequency, and server_advertise is true, this change adds the directive AdvertiseFrequency in mod_cluster virtual host conf file template

@tphoney
Copy link
Contributor

tphoney commented Mar 8, 2017

Looks like a good change, it might be worth documenting this new parameter in the readme.

@EmersonPrado
Copy link
Contributor Author

@tphoney Good idea. Done.

@tphoney
Copy link
Contributor

tphoney commented Mar 9, 2017

in the docs you say the default value is 10, but it in code it is undef. its late here, so i might be code blind

@wilson208
Copy link
Contributor

@tphoney have looked at the docs here and AdvertiseFrequency defaults to 10ms when not defined. This PR only sets the AdvertiseFrequency in the template when the param is defined, therefore it defaults to 10ms when not defined.

This LGTM 👍

@wilson208 wilson208 merged commit ef29dc6 into puppetlabs:master Mar 10, 2017
@EmersonPrado
Copy link
Contributor Author

@wilson208 and @tphoney : would it be overkill to push a new commit improving the README to explain the default is from Apache modcluster, not from the Puppet class?
(Just for pickiness: 10s, not 10ms)

cegeka-jenkins pushed a commit to cegeka/puppet-apache that referenced this pull request Jul 15, 2020
Modules-4500 Add optional "AdvertiseFrequency" directive in cluster.conf template
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