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

Include apache::params in apache::mod::php #102

Merged
merged 1 commit into from
Oct 15, 2012
Merged

Include apache::params in apache::mod::php #102

merged 1 commit into from
Oct 15, 2012

Conversation

reidmv
Copy link
Contributor

@reidmv reidmv commented Oct 15, 2012

The apache::mod::php class makes reference to a params variable but does not explicitely include the apache::params class. Parse order can result in not having the appropriate variable available and a file /php5.conf being created and managed. This commit resolves the problem by adding an explicity include apache::params line to apache::mod::php.

The apache::mod::php class makes reference to a params variable but does
not explicitely include the apache::params class. Parse order can result
in not having the appropriate variable available and a file /php5.conf
being created and managed. This commit resolves the problem by adding an
explicity `include apache::params` line to apache::mod::php.
@hunner
Copy link
Contributor

hunner commented Oct 15, 2012

I guess it was assumed that the base apache class would be included 100% of the time that any of the sub-classes or defined resources are used, but okay.

hunner added a commit that referenced this pull request Oct 15, 2012
Include apache::params in apache::mod::php
@hunner hunner merged commit fcb67fe into puppetlabs:master Oct 15, 2012
@reidmv
Copy link
Contributor Author

reidmv commented Oct 15, 2012

In the case where I was running into problems the base apache class was never explicitly included in user manifests. The user manifests simply made use of apache::vhost defined resources which when parsed will include apache. So the following would have been valid but would have broken:

class role::foo {
  include apache::mod::php

  apache::vhost { '*':
    priority => '00',
    port     => '80',
    docroot  => '/srv/www/default',
  }

}

@hunner
Copy link
Contributor

hunner commented Oct 15, 2012

Yeah. This goes back to whether using any <module>::* pieces should require include <module> base class for bootstrapping or not. But the path-of-least-surprise also applies.

@hunner
Copy link
Contributor

hunner commented Oct 18, 2012

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.

None yet

3 participants